-
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
Build: upgrade mini-css-extract-plugin-with-rtl to npm-published a8c fork #38001
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
91658e3
to
71529a2
Compare
@blowery I'm unable to update the Even if I remove the root
It seems that referencing packages outside the current NPM root, e.g., Apparently, you were able to produce valid lockfiles in one of earlier version of this PR. How did you do that? 🙂 |
The version of this patch currently in this branch has all the That's also why the CI tasks completely fail. |
@jsnajdr here's some things to try:
|
This is the step where I'm failing. An important point is that I changed the reference to Then the lockfile doesn't contain any |
What version of npm? Does |
I'm on npm 6.9.0, the one that ships with the current Node LTS. Nothing special. |
71529a2
to
ebd05c2
Compare
@jsnajdr redid the locks here with |
ebd05c2
to
72259e8
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! Tested out well.
Updated the
mini-css-extract-plugin-with-rtl
package as follows:mini-css-extract-plugin
. The original fork repo uses a rather chaotic set of patch and merge commits that's difficult to rebase and maintain.v0.8.0
version ofmini-css-extract-plugin
@automattic/mini-css-extract-plugin-with-rtl@0.8.0
Then I updated the
calypso-build
package to use the npm-published package.Because version 0.8.0 introduced a new
ignoreOrder
option that allows to disable the conflicting order warnings, we no longer need to disable them ourselves withwebpack-filter-warnings-plugin
and can remove that code and thefilter-warnings
plugin, too, as this was its only usage.Then I also updated the lockfiles of
apps/full-site-editing
andapps/wpcom-block-editor
.full-site-editing
referenced npm-published@automattic/calypso-build
, so I changed that to afile:
reference: makes more sense for monorepo.How to test:
Build and run Calypso with
?flags=quick-language-switcher
. Verify that when switching between LTR and RTL languages, the CSS chunks are switched at runtime.