Skip to content

Add sideEffects directive to all package.json#9961

Merged
aduth merged 6 commits intomainfrom
aduth-pkg-side-effects
Feb 7, 2024
Merged

Add sideEffects directive to all package.json#9961
aduth merged 6 commits intomainfrom
aduth-pkg-side-effects

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jan 23, 2024

🛠 Summary of changes

Adds (and enforces) the presence of a sideEffects directive within each package's package.json.

Why?

By default, Webpack and other common bundlers assume that any imported code may include side effects, and as such will not aggressively try to perform dead code elimination. Explicitly defining side effects (or the absence of them) allows Webpack to perform this optimization.

See also:

In practice, I noticed this by looking at the platform-authenticator-avaiable-*.js file loaded on https://secure.login.gov/ , which includes the snippet of code new Set(["NotAllowedError", "TimeoutError", "InvalidStateError"]); irrelevant for that bundle.

Performance Impact

yarn build:js

Total JavaScript size:

du -s public/packs/js

Before: 12240kb
After: 12176kb
Diff: -64kb (-0.5%)

Critical path (http://localhost:3000), with gzip enabled:

Before: 6.4kb
After: 6.3kb
Diff: -0.1kb (-1.6%)

Observation: The benefit here isn't as substantial as I might have hoped, and may not warrant the effort involved in maintaining an accurate configuration for every package. I could be open to pushback here. I still think targeting a lack of side effects is ideal, with custom elements being the main point of complication. The savings here could be more substantial in the future depending on future package code revisions.

📜 Testing Plan

  1. rm -rf public/packs/js
  2. NODE_ENV=production yarn build:js
  3. cat public/packs/js/platform-authenticator-available-*.digested.js | grep NotAllowedError
  4. echo $?
  5. Observe: "1"

@aduth aduth marked this pull request as ready for review January 24, 2024 14:29
@aduth aduth force-pushed the aduth-pkg-side-effects branch from de6b69f to 335ebf5 Compare January 31, 2024 13:40
@aduth aduth force-pushed the aduth-pkg-side-effects branch from 335ebf5 to e341be9 Compare February 7, 2024 13:12
@aduth aduth merged commit c50e219 into main Feb 7, 2024
@aduth aduth deleted the aduth-pkg-side-effects branch February 7, 2024 15:11
@mitchellhenke mitchellhenke mentioned this pull request Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants