Micro-optimize bundle size of PasswordToggleElement#7673
Conversation
micro-optimization: transpilation of args defaulting is slightly verbose. also results in marginal reduction of size of payload over the wire (see spec changes)
micro-optimization: shave a few characters "window"
micro-optimization: avoid error variable and "try' statement; minified output is also better able to collapse the preceding "if" statement
- more aligned with standards vs. object of elements - less indirection, more direct access - avoid decorator premature optimization - micro-optim: avoid "elements" pass-through changelog: Internal, Performance, Reduce size of JavaScript bundles
| * @return Promise resolving once event has been logged. | ||
| */ | ||
| export async function trackEvent(event: string, payload: object = {}): Promise<void> { | ||
| export async function trackEvent(event: string, payload?: object): Promise<void> { |
There was a problem hiding this comment.
For context, argument defaulting transpilation is a bit verbose. I expect this would correct itself once browser support for optional arguments is good enough to avoid the syntax downgrading.
Transpiled result:
async function(e) {
let n = arguments.length > 1 && void 0 !== arguments[1] ? arguments[1] : {};
// ...
}
There was a problem hiding this comment.
My first thought was, why are we transpiling this? but wow default parameters only have 93% availability on canicuse, that is surprising
There was a problem hiding this comment.
That is surprising!
There are ways we could optimize this a bit by telling Babel certain "assumptions", but I'm not overly stressed about it broadly speaking. Hopefully with having dropped IE support and targeting browser support percentage, it won't be too long before we can use the native syntax.
| * @return Promise resolving once event has been logged. | ||
| */ | ||
| export async function trackEvent(event: string, payload: object = {}): Promise<void> { | ||
| export async function trackEvent(event: string, payload?: object): Promise<void> { |
There was a problem hiding this comment.
My first thought was, why are we transpiling this? but wow default parameters only have 93% availability on canicuse, that is surprising
|
|
||
| def valid_payload? | ||
| !log_params[:payload].nil? | ||
| params[:payload].nil? || !log_params[:payload].nil? |
There was a problem hiding this comment.
I think we should keep the server strict about these
There was a problem hiding this comment.
The payload is optional for both the Ruby and JavaScript versions of this API. Any reason it shouldn't also be optional for the JSON API?
There was a problem hiding this comment.
I guess that's right, but all events have extra attributes. I take my comment back
There was a problem hiding this comment.
I'm not super happy with how the validation turned out here, since how Rails permit handling with a hash makes it so all invalid parameters (including a nil value) would be normalized to an empty hash, so it requires this juggling of both params and #permit'd log_params to allow omission while still rejecting anything other than a hash.
| await fetch(endpoint, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ event, payload }), |
There was a problem hiding this comment.
what if we used || to do what the default value did? it's still an overall character reduction right?
| body: JSON.stringify({ event, payload }), | |
| body: JSON.stringify({ event, payload: payload || {} }), |
There was a problem hiding this comment.
what if we used
||to do what the default value did? it's still an overall character reduction right?
Yeah this would technically work and be reasonably effective at reducing overall size. To my other comment though, I guess I don't see the issue with omitting the payload if we have nothing to send.
2 participants
🛠 Summary of changes
Minor refactoring of a few files to simplify and optimize the size of JavaScript bundles, particularly the PasswordToggleElement implementation and its dependencies, toward a goal of reducing overall JavaScript size in critical paths (notably, the Sign In page).
For more detail, refer to individual commits for specific changes and rationale.
Performance Results
Reduces gzipped size of Password Toggle bundle from 933b to 558b (-40.2%).
We're starting to hit a point of diminishing returns, since even though 40.2% reduction sounds impressive, we're talking an absolute difference of about 0.4kb 😅