-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dynamically load jQuery on the desktop #27908
Conversation
This is an automated check which relies on Generated by 🚫 dangerJS |
bff737d
to
5eb8531
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -32,7 +30,7 @@ const config = { | |||
[ '@babel/plugin-proposal-class-properties', { loose: true } ], | |||
[ '@babel/plugin-transform-classes', { loose: false } ], | |||
[ '@babel/plugin-transform-template-literals', { loose: true } ], | |||
isCalypso && [ | |||
[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @vindl! @jsnajdr @dmsnell @ockham @sirreal I changed the babel config as part of this to always include the asyncRequire / AsyncLoad transform. It was previously not being run on test code, which prevented us from testing code that uses asyncRequire. Does anyone know why we were restricting that transform before? |
Not me but it seems like a reasonable assumption that it would be web-only since that's where the async constraint comes in. I'm fine removing it - thanks for this update! |
I'd speculate, like @dmsnell, that this was because the Desktop app supposes it has access to the entire app immediately and doesn't need to request bundles over the network as the app loads. wp-calypso/config/desktop.json Line 24 in 3da9faf
With that in mind, it doesn't seem like the |
PR #27778 ended up pulling jQuery into the vendor~build bundle due to a conditional require in lib/load-script. Get it back out of build by dynamically loading jQuery on desktop. Webpack will still see this require on the normal web build as a require.ensure and build a chunk for it, which we'll probably never load.