Build: Fix imported modules breaking ie11#12463
Conversation
|
Caution: This PR has changes that must be merged to WordPress.com |
|
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: June 4, 2019. |
jeherve
left a comment
There was a problem hiding this comment.
My IE11 VM won't start anymore, so I could only test in a different browser. I can confirm that the dashboard is displayed nicely and analytics still work.
I'll let someone else test in IE11.
brbrr
left a comment
There was a problem hiding this comment.
Dashboard works fine now!
There is quite a problem with "My Plan" page though, which I think is not related to this PR.
Logged in #12464 |
* Kick off the changelog * Add 7.3.1 * Update date and post link * changelog: add #12219 * changelog: add #12170 * changelog: add #12184 * Changelog: add #12268 * Changelog: add #12081 * Changelog: add #12323 * Changelog: add #12204 * Changelog: add #12269 * Changelog: add #12332 * changelog: add #12339 * changelog: add #12209 * Changelog: add #12319 * Changelog: add #12357 * Changelog: add #12124 * Changelog: add #12373 * Changelog: add #12252 * Changelog: add #12383 * Changelog: add #12372 * changelog: add #12337 * Changelog: add #12290 * Changelog: add #12301 * Changelog: add #12061 * Testing list: add instructions for #12061 * Changelog: add #12393 * Update minimum supported version See #12287 * Changelog: add #12406 * Testing list: add #12406 * Changelog: add #12277 * Changelog: add #12412 * Changelog: add #11318 * Changelog: add #12328 * Changelog: add #12425 * Changelog: add #12380 * Changelog: add #12428 * Changelog: add #12414 * Changelog: add #12395 * Changelog & Testing list: add #12416, #12417, #12418, and #12348 * changelog: add #12379 * Changelog: add #12341 * changelog: add #12444 * Changelog: add #12434 * Changelog: add #12454 * Changelog: add #12460 * Changelog: add #12463 * Changelog: add #12457 * Changelog / testing list: add #10333 * Changelog: add #12467 Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
There's no point to it, the generated comments just bloat the CSS and license.txt files. Note Webpack already defaults it to true for `mode: development` builds, where it makes a little more sense for the comments to be present. It's not clear why it was ever being set in the first place, neither #12463 nor #13070 seem to have any discussion about the additions.
…21727) There's no point to it, the generated comments just bloat the CSS and license.txt files. Note Webpack already defaults it to true for `mode: development` builds, where it makes a little more sense for the comments to be present. It's not clear why it was ever being set in the first place, neither #12463 nor #13070 seem to have any discussion about the additions.
Fixes #12407
There were two issues:
1 - config module
Use local
configmodule overnode_modules/config:. #12403 addedconfigdependency which is imported here:jetpack/_inc/client/lib/analytics/index.js
Line 10 in 597bc7c
Due to module loading here:
jetpack/webpack.config.js
Line 24 in 597bc7c
The new
node_modules/configmodule is used over the intended one. That introduced new ie11 errors. Fixed here by importing the module with a relative path.We might consider ordering the module resolution so that local modules have preference.
2 - debug module
The debug module is used directly in the application:
jetpack/_inc/client/lib/analytics/index.js
Line 4 in 597bc7c
This dependency was not declared in
package.json, so the whatever version happened to be available as a transitive dependency was used. v4 appears to be incompatible with ie11 and webpack/babel compilation and it is not used in Calypso, likely for this reason.Fix by adding
debug@3^as a dependency.We should enable an eslint lint rule like
'import/no-extraneous-dependencies': [ 'error', { packageDir: './' } ]to help prevent errors like this.Testing instructions:
Try loading
/wp-admin/admin.php?page=jetpackfor a Jetpack site in IE11. Smoke test. The page should work properly without errors.Proposed changelog entry for your changes: