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

chore(webpack): simplify our webpack config #11099

Merged
merged 18 commits into from
May 10, 2024
Merged

chore(webpack): simplify our webpack config #11099

merged 18 commits into from
May 10, 2024

Conversation

LeoMcA
Copy link
Member

@LeoMcA LeoMcA commented May 8, 2024

Summary

https://mozilla-hub.atlassian.net/browse/MP-1132

Problem

As a result of using create react app, our webpack config is over complicated.

Solution

Begin simplifying it. None of these changes aside from the first commit should make a difference to our build output, which can be verified following the instructions in the section below.

chore(webpack): deterministic output when changing config does make a change to our build output, but this is to ensure the other changes don't. A full explanation is in the commit message, but the summary is that due to internal webpack naming logic, changes to the webpack config which shouldn't have made any difference were changing the computed names for modules and chunks.


How did you test this change?

Checkout chore(webpack): deterministic output when changing config, and build the client, moving the assets into another folder:

git checkout 4a5cf81b8958bb3b5f80b8d20ae0bc1ae1d426bb
yarn && yarn build:client
mv client/build client/build.main

We now effectively have the webpack output from main, plus the changes in chore(webpack): deterministic output when changing config necessary to ensure changes to webpack which don't change the output actually don't change the output (due to the aforementioned internal webpack naming logic). Now checkout this PR and build it:

git checkout webpack-cleanup
yarn build:client
diff -r client/build/ client/build.main/

The diff should exit with no output, showing the webpack output hasn't changed.

LeoMcA added 5 commits May 8, 2024 16:39
found through much testing, setting chunkIds and moduleIds to
"deterministic" (which is the default in production) results in
undeterministic behavior when making changes to the webpack config which
don't result in any changes to the inherent content of processed files

this seems to be because "deterministic" uses metadata about all
configured module loaders there are in the webpack config when
calculating the hash, so if you remove a loader which isn't doing
anything, that change still results in a change to the module and chunk
ids

instead, using either "named" or "natural" ensures that such changes
don't result in any changes to the webpack output

this allows easy verification that modifications to the webpack config
haven't changed any output with a simple diff
we don't use it due to our CSP
@LeoMcA LeoMcA requested review from mdn-bot and a team as code owners May 8, 2024 17:06
@LeoMcA LeoMcA force-pushed the webpack-cleanup branch from fde6a11 to 0b9ebf3 Compare May 8, 2024 17:18
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label May 9, 2024
Comment on lines -332 to -333
// TODO: Merge this config once `image/avif` is in the mime-db
// https://github.com/jshttp/mime-db
Copy link
Contributor

Choose a reason for hiding this comment

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

So satisfactory when a long game finally resolves!

Copy link
Contributor

@argl argl left a comment

Choose a reason for hiding this comment

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

This looks great! Successfully tested locally, astonishing amount of cruft removed.

@LeoMcA LeoMcA merged commit 13c468d into main May 10, 2024
16 checks passed
@LeoMcA LeoMcA deleted the webpack-cleanup branch May 10, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants