Use webpack config JS package in Jetpack #21581

anomiex · 2021-10-28T20:15:58Z

Changes proposed in this Pull Request:

This shouldn't change anything visible in this iteration, but double-checking
might be a good idea.

This does take the opportunity to fix the extension build to keep
plugins and blocks in the order defined in extensions/index.json,
where #20994 had put all the blocks before all the plugins.

This also replaces the uses of @automattic/calypso-build's jest configuration with local config.

Jetpack product discussion

PT: p9dueE-3MG-p2

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Does it build?
Is CI happy?
See that things touched here still work right.

This is intended to (eventually) take care of several issues in the monorepo: * Calypso [doesn't use their own calyspo-build webpack config][1], which means it's not likely to be well-maintained. * It needs a hack to [work with monorepo packages][2]. * And another to [get correct image paths][3]. * And it [tries to redefine NODE_ENV, producing warnings][4]. * And we have to [override a weird default everywhere][5]. * We have to [name our JS files with ".min.js"][6] to avoid their being broken by a wpcom minifier, but WordPress.org's translation infrastructure [ignores such files][7]. * The plugin used to fix the above [had to be forked][8], adds 772K to the plugin, and results in spurious changes in TC builds. * The way translations are [being loaded for lazy-loaded bundles][9] is pretty hacky and doesn't lazy-load them. * We can't use any cache busting for the lazy-loaded bundles, because embedding hashes in the filename breaks the link to the translation file and putting it in a query parameter [breaks building the RTL css][10]. * Webpack's minification is [losing translator comments][11]. * Calypso-build has a lot of peer dependencies we don't actually need. Not everything we have uses sass or postcss, and we have our own infrastructure for jest, react, and so on. Currently we just ignore the 76 warnings from that. This is the first step towards fixing that: creating a private package that can hold the webpack and babel config fragments that we can share throughout the monorepo. Then we'll have a place to put the fixes for some of the other issues too. [1]: #21004 (comment) [2]: Automattic/wp-calypso#53353 [3]: Automattic/wp-calypso#56111 [4]: Automattic/wp-calypso#56291 [5]: #21004 [6]: #20484 [7]: #21343 [8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35 [9]: #20926 [10]: #21349 [11]: #16549

Mostly as a proof of concept. But I also took the opportunity to clean a few things up: * Name the built JS as ".min.js" like it should be. * Use copy-webpack-plugin to copy the src version of intersection-observer. * Use @wordpress/dependency-extraction-webpack-plugin to handle the cache-buster version in `wp_enqueue_script()`.

I tried not to change too much in this iteration, but I do note that the new package uses a newer version of @wordpress/browserslist-config than calyspo-build does so it now transpiles without IE11 support.

I tried not to change too much in this iteration, but I do note that the new package uses a newer version of @wordpress/browserslist-config than calyspo-build does so it now transpiles without IE11 support. Also I dropped babel-minify-webpack-plugin, as terser that's included with webpack seems to work just as well (and has been updated more recently than 4 years ago).

This results in no change to the production build. The development build is no longer actually minified.

This also reverts the "bring back the "Cloud" icon, the path changed after the Webpack update" bit from #20900. The problem was Automattic/wp-calypso#56111, which is not present in the new package.

This shouldn't change anything in this iteration, but double-checking might be a good idea. This does take the opportunity to fix the extension build to keep plugins and blocks in the order defined in extensions/index.json, where #20994 had put all the blocks before all the plugins.

Apparently it was somehow still finding it so the tests didn't fail.

github-actions · 2021-10-28T20:17:06Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: December 7, 2021.
Scheduled code freeze: November 30, 2021.

Backup plugin:

Next scheduled release: December 7, 2021.
Scheduled code freeze: November 29, 2021.

…fig-js-package-in-jetpack

sdixon194

Things seem to be fine here. Can't speak to the webpack code itself, but functionally all seems okay.

…fig-js-package-in-jetpack

matticbot · 2021-11-05T19:50:37Z

Caution: This PR has changes that must be merged to WordPress.com
Hello anomiex! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D69704-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

anomiex · 2021-11-05T20:05:41Z

Needs a fresh approval thanks to a merge conflict.

jsnmoon

Changes look reasonable and the builds work without issue. All Search products appear to be unaffected by the change.

👍

github-actions · 2021-11-08T14:56:12Z

Great news! One last step: head over to your WordPress.com diff, D69704-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

anomiex · 2021-11-08T15:06:03Z

r234651-wpcom

# By renovate[bot] (3) and others # Via GitHub * master: Update dependency preact to v10.5.15 (#21635) Search: Migrate helper classes to standalone package (#21552) Release script: fix small typo (#21675) Update babel monorepo (#21668) Lock file maintenance (#21664) E2E tests: clean unused dependencies (#21639) Create JS Config package (#21592) Use webpack config JS package in Jetpack (#21581) add test runner to dev dependencies (#21591) Pexels Integration: Make the initial image search for example query run only once (#21641) Boost: make progress bar corners round on small screens (#21626) Full Sync :: limit to contributors and above (#21648) Sync Checksums : limit users based on user_level (#21645) Storybook: add Jetpack Spinner (#21511) Contact Info and Map widget: widget deprecation (#21554) Use webpack config js package in everything except Jetpack (#21580) # Conflicts: # pnpm-lock.yaml # projects/plugins/boost/package.json

* master: (24 commits) Update dependency preact to v10.5.15 (#21635) Search: Migrate helper classes to standalone package (#21552) Release script: fix small typo (#21675) Update babel monorepo (#21668) Lock file maintenance (#21664) E2E tests: clean unused dependencies (#21639) Create JS Config package (#21592) Use webpack config JS package in Jetpack (#21581) add test runner to dev dependencies (#21591) Pexels Integration: Make the initial image search for example query run only once (#21641) Boost: make progress bar corners round on small screens (#21626) Full Sync :: limit to contributors and above (#21648) Sync Checksums : limit users based on user_level (#21645) Storybook: add Jetpack Spinner (#21511) Contact Info and Map widget: widget deprecation (#21554) Use webpack config js package in everything except Jetpack (#21580) Boost: Fix the official name to the JavaScript word reference (#21638) tools: Allow extra spaces in next-version tags (#21647) Publicize: always add an alt text to Open Graph Images (#21520) js-tools: Add missing eslint plugin dep (#21642) ...

anomiex added 8 commits October 28, 2021 15:02

connection-ui: Use the new webpack-config package

e1907dc

I tried not to change too much in this iteration, but I do note that the new package uses a newer version of @wordpress/browserslist-config than calyspo-build does so it now transpiles without IE11 support.

jitm: Use the new webpack-config package

aba00f3

This results in no change to the production build. The development build is no longer actually minified.

backup: Use the new webpack-config package

93e2940

This also reverts the "bring back the "Cloud" icon, the path changed after the Webpack update" bit from #20900. The problem was Automattic/wp-calypso#56111, which is not present in the new package.

jetpack: Remove uses of calyspo-build in jest config too

cc53b88

Apparently it was somehow still finding it so the tests didn't fail.

anomiex added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Janitorial [Pri] Normal labels Oct 28, 2021

anomiex self-assigned this Oct 28, 2021

anomiex requested a review from a team as a code owner October 28, 2021 20:15

matticbot added the Touches WP.com Files label Oct 28, 2021

github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ Instant Search [Feature] Search For all things related to Search labels Oct 28, 2021

This was referenced Oct 29, 2021

webpack-config: Fork calypso-build's mini-css-with-rtl plugin #21595

Merged

Create JS Config package #21592

Merged

Base automatically changed from add/use-webpack-config-js-package-in-everything-except-jetpack to master November 5, 2021 14:27

Merge remote-tracking branch 'origin/master' into add/use-webpack-con…

19011dc

…fig-js-package-in-jetpack

sdixon194 previously approved these changes Nov 5, 2021

View reviewed changes

Merge remote-tracking branch 'origin/master' into add/use-webpack-con…

a7d423c

…fig-js-package-in-jetpack

anomiex dismissed sdixon194’s stale review via a7d423c November 5, 2021 19:49

jsnmoon approved these changes Nov 5, 2021

View reviewed changes

sdixon194 added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Nov 8, 2021

anomiex merged commit 89aeaac into master Nov 8, 2021

anomiex deleted the add/use-webpack-config-js-package-in-jetpack branch November 8, 2021 14:55

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 8, 2021

anomiex mentioned this pull request Nov 9, 2021

Search: try to move search dashboard to search package #21442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use webpack config JS package in Jetpack #21581

Use webpack config JS package in Jetpack #21581

anomiex commented Oct 28, 2021 •

edited

Loading

github-actions bot commented Oct 28, 2021 •

edited

Loading

sdixon194 left a comment

matticbot commented Nov 5, 2021

anomiex commented Nov 5, 2021

jsnmoon left a comment

github-actions bot commented Nov 8, 2021

anomiex commented Nov 8, 2021

Use webpack config JS package in Jetpack #21581

Use webpack config JS package in Jetpack #21581

Conversation

anomiex commented Oct 28, 2021 • edited Loading

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented Oct 28, 2021 • edited Loading

sdixon194 left a comment

Choose a reason for hiding this comment

matticbot commented Nov 5, 2021

anomiex commented Nov 5, 2021

jsnmoon left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 8, 2021

anomiex commented Nov 8, 2021

anomiex commented Oct 28, 2021 •

edited

Loading

github-actions bot commented Oct 28, 2021 •

edited

Loading