♻️ Reduce size of interactive extension #30233
Conversation
|
Hey @gmajoulet! These files were changed: |
|
So no updates to the store service are needed? The service class itself isn't duplicated across extension builds? |
|
The problem is how the array is being used. From the compiler standpoint, if you access a const object with the square brackets, it will need to import the whole object as the keys are calculated in runtime. If you access the keys with the dot notation, the compiler optimizes that and the compiled code ends up being The services class is only used with the static functions so the functions are compiled and substituted in place. The Services class doesn't get bundled. Only weird thing I'm investigating is that it's including the experiments.js file... |
|
Update: experiments are part of core amp and are embedded into all the extensions. I feel like this should not happen, but it would require a lot of digging to remove it safely |
| /** @private {!Array<string>} */ | ||
| this.answerChoiceOptions_ = ['A', 'B', 'C', 'D']; | ||
| /** @private {!Array<../../../src/localized-strings.LocalizedStringId>} */ | ||
| this.answerChoiceOptions_ = [ |
There was a problem hiding this comment.
Nit: let's update this variable name to reflect what's inside it. I know you mutate it later, which makes it even more confusing because the type is no longer correct
There was a problem hiding this comment.
That's true, technically the type is always a string but pr-check required it to be this type in the JSDoc. I'll change the name so that it shows it can be the localized values
There was a problem hiding this comment.
If having two variables (one for the localizedIds and one for the strings) make things easier to understand and more accurate, we should go for it
There was a problem hiding this comment.
Alternately, we could construct the localizedIds array inline, right before sending them through the localization service.
There was a problem hiding this comment.
I don't feel the need to have two variables, since they just clutter the component (many components have so many variables that it's hard to understand what they do). Because of the type inconsistency, constructing the array in-line makes the most sense, as the LocalizedStringId keys won't be used anywhere else. Sent the commit with it
* Reduced quizzes size by 8.5kb * Reformatted code * Removed unused dev * Fixed type conversion * Renamed answerChoices * Moved localizedStringIds to inline
Managed to reduce size of interactive components significantly:
69.270kb -> Before
65.420kb -> After removing the unnecessary import of all LocalizedIds, by refactoring the uses.
61.778kb -> After simplifying quiz CSS, check https://codepen.io/mszylkowski/pen/BaKPppN to see new animation
These changes decrease the size of the extension by 11%
Closes #29282