Use webpack config js package in everything except Jetpack #21580

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

Changes proposed in this Pull Request:

backup: Use the new webpack-config package

This also reverts the "bring back the "Cloud" icon, the path changed
after the Webpack update" bit from Backup: fix dashboard styling #20900. The problem was
Jetpack: Calypso Build Breaks Image Paths From Incorrect publicPath wp-calypso#56111, which is not present in the new package.
jitm: Use the new webpack-config package

This results in no change to the production build. The development build
is no longer actually minified.
identity-crisis: Use the new webpack-config package

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).
connection-ui: Use the new webpack-config package

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.
lazy-images: Use the new webpack-config package

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().

Jetpack product discussion

PT: p9dueE-3MG-p2

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

No.

Testing instructions:

Do they build?
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.

github-actions · 2021-10-28T20:16:29Z

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-everything-except-jetpack

matticbot · 2021-11-02T13:45:15Z

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 D69424-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

…fig-js-package-in-everything-except-jetpack

sdixon194 · 2021-11-03T18:56:50Z

Tried to test this one pretty thoroughly and ran into some issues when testing on a JN site, (but not when testing and building locally). These issues weren't present when running bleeding edge for both Backup and Jetpack:

Backup

When disconnected from Jetpack and going to Backup's wp-admin page to connect, it seems the image isn't loading:

On this branch:

On Bleeding Edge:

Lazy Loading

When lazy loading is enabled, I'm getting this PHP error:

Warning: filemtime(): stat failed for https://lucky-warbler.jurassic.ninja/wp-content/plugins/jetpack-dev/vendor/automattic/jetpack-lazy-images/src/../dist/intersection-observer.min.js in /srv/users/user18ca27f6/apps/user18ca27f6/public/wp-content/plugins/jetpack-dev/vendor/automattic/jetpack-lazy-images/src/lazy-images.php on line 492

As far as I could see, JITMs and other Jetpack functionality seems to work fine. Didn't test IDC.

Again, no longer needed now what we're not hitting that calypso-build bug.

…fig-js-package-in-everything-except-jetpack

anomiex · 2021-11-04T18:14:09Z

Good testing, thanks! Both of those should be fixed now.

sdixon194

Nice! This all works for me now.

github-actions · 2021-11-05T14:28:59Z

Great news! One last step: head over to your WordPress.com diff, D69424-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-05T14:29:20Z

r234590-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 6 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.

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

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

Base automatically changed from add/webpack-config-js-package to master November 1, 2021 21:06

github-actions bot added [JS Package] Webpack Config RNA labels Nov 1, 2021

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

d40c0db

…fig-js-package-in-everything-except-jetpack

anomiex added 2 commits November 2, 2021 10:09

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

5fd5453

…fig-js-package-in-everything-except-jetpack

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

da98fe5

…fig-js-package-in-everything-except-jetpack

anomiex mentioned this pull request Nov 3, 2021

Create JS Config package #21592

Merged

sdixon194 removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Nov 3, 2021

sdixon194 added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Nov 3, 2021

anomiex added 4 commits November 4, 2021 13:27

lazy-images: Fix filemtime call

8907ab5

backup: Remove the last bits of assetBuildUrl

bf92fa1

Again, no longer needed now what we're not hitting that calypso-build bug.

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

de082a6

…fig-js-package-in-everything-except-jetpack

How'd I miss that?

2c3d760

anomiex added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Nov 4, 2021

lazy-images: Fix for real this time

3c10550

sdixon194 approved these changes Nov 4, 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 4, 2021

anomiex merged commit 866fc56 into master Nov 5, 2021

anomiex deleted the add/use-webpack-config-js-package-in-everything-except-jetpack branch November 5, 2021 14:27

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

anomiex mentioned this pull request Nov 8, 2021

My Jetpack: Add initial skeleton #21464

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use webpack config js package in everything except Jetpack #21580

Use webpack config js package in everything except Jetpack #21580

anomiex commented Oct 28, 2021 •

edited

Loading

github-actions bot commented Oct 28, 2021 •

edited

Loading

matticbot commented Nov 2, 2021

sdixon194 commented Nov 3, 2021

anomiex commented Nov 4, 2021

sdixon194 left a comment

github-actions bot commented Nov 5, 2021

anomiex commented Nov 5, 2021

Use webpack config js package in everything except Jetpack #21580

Use webpack config js package in everything except Jetpack #21580

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

matticbot commented Nov 2, 2021

sdixon194 commented Nov 3, 2021

anomiex commented Nov 4, 2021

sdixon194 left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 5, 2021

anomiex commented Nov 5, 2021

anomiex commented Oct 28, 2021 •

edited

Loading

github-actions bot commented Oct 28, 2021 •

edited

Loading