Skip to content
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

CPP-2230: upgrade to Webpack 5 #1002

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

CPP-2230: upgrade to Webpack 5 #1002

wants to merge 28 commits into from

Conversation

apaleslimghost
Copy link
Member

@apaleslimghost apaleslimghost commented Sep 27, 2023

updates Page Kit's Webpack plugins to support Webpack 5. not backwards compatible with Webpack 4; most apps will be able to update without changing their config (although there are some things like raw-loader in use that v5 has more modern ways of doing that would be good to adopt).

i've tested this with the kitchen-sink example and next-article and everything seems to be working. it probably warrants more testing. it's a little annoying to link everything together to test locally; as well as the Page Kit packages you need to link the app to Page Kit's installs of at least webpack and react/react-dom.

webpack 5 breaking changes apps need to be aware of

  • when a package has an exports specifier in package.json Webpack 5 is strict about not importing things that aren't exported in exports. this may require code changes in packages and/or apps to export everything that should be and import things correctly.
    • for example, next-article imports @financial-times/custom-code-component/dist/custom-code-component.js, but that package doesn't include that entrypoint in its exports (it exports ., the top-level import, pointing that at the file dist/custom-code-component.js, which isn't visible outside the package). next-article won't build with Webpack 5 without changing that import to @financial-times/custom-code-component.

spicy changes i'm making in this PR

  • removing Autoprefixer. the CSS we write is supported in every browser we use, and browsers no longer use vendor prefixes for new features. the only holdouts are a handful of -webkit prefixed properties supported by every browser; i have verified via Github search that we never use any of these properties unprefixed. this will eventually allow us to move away from PostCSS, speeding up our builds somewhat.
  • switching our Browserslist config for @babel/preset-env from attempting to codify our browser support to just using Browserslist defaults. defaults is industry best practice, and slightly more lenient than our current browser support policy. this speeds up our build and reduces bundle size (by reducing how much syntax we need to compile), and reduces maintenance burden of updating our Browserslist config when our browser support policy changes.

@apaleslimghost apaleslimghost requested a review from a team as a code owner September 27, 2023 14:42
@apaleslimghost apaleslimghost requested a review from a team as a code owner October 7, 2024 15:40
@apaleslimghost apaleslimghost force-pushed the webpack-5 branch 2 times, most recently from 688ffb5 to cf18e50 Compare October 14, 2024 10:21
@apaleslimghost apaleslimghost changed the title WIP: upgrade to Webpack 5 CPP-2230: upgrade to Webpack 5 Oct 17, 2024
@apaleslimghost
Copy link
Member Author

hmm. the build is failing because npm ci takes >11m to complete. don't know why it would be taking so long

also remove disabling cssnano's reduceInitial because we don't support ie11
…t default browsers

`defaults` is browserslist best practice and is slightly more lenient than our current browser support
our browser support long ago outpaced how modern our CSS is, and browsers no longer use vendor prefixes for new features.
@@ -45,42 +46,21 @@ export class PageKitSassPlugin {
// Disable formatting so that we don't spend time pretty printing
outputStyle: 'compressed',
// Enable Sass to @import source files from installed dependencies
includePaths: ['bower_components', 'node_modules', ...this.includePaths]
includePaths: [
'bower_components',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: probably safe to remove this as a path at this point 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants