perf: make the published package tree-shakeable - #1841
Open
saurabhraghuvanshii wants to merge 4 commits into
Open
saurabhraghuvanshii wants to merge 4 commits into
saurabhraghuvanshii wants to merge 4 commits into
Conversation
Contributor
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe package metadata and bundler configuration now use explicit ESM and dependency handling. ESLint rejects MUI icon barrel imports. Source files use dedicated MUI and lodash paths plus namespace imports for ChangesModule resolution updates
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Consumers could not tree-shake @sistent/sistent at all. Importing one
component from 0.22.6 pulls ~14,000 modules and 14.7 MB into a webpack build,
10,775 of those modules being @mui/icons-material. In layer5.io this was the
largest single contributor to build memory.
Three causes, all fixed here:
- `module` pointed at the CommonJS build. `main` and `module` were both
`./dist/index.js`, so no bundler ever selected `dist/index.mjs`. `module`
now points at the ESM build, and an `exports` map routes `import` and
`require` to the matching build and declarations.
- There was no `sideEffects` field, so bundlers had to keep every export.
The package has no top-level side effects (no CSS imports, no global
registration), so it is now marked `sideEffects: false`.
- Six components imported icons from the `@mui/icons-material` barrel,
which re-exports ~10,800 modules. They now import by path, and a
`no-restricted-imports` rule keeps the barrel out.
Serving the ESM build exposed two imports that only worked through CJS
interop, and would have broken strict ESM consumers:
- `import jsyaml from 'js-yaml'` has no default export in js-yaml's ESM
build; now `import * as jsyaml`.
- `lodash/debounce` has no extension and lodash has no exports map, which
webpack rejects from an `.mjs` file (`fullySpecified`); now
`lodash/debounce.js`.
Measured with webpack 5, a consumer that imports only `Box`:
published 0.22.6 this change*
modules 14,132 5,288
@mui/icons-material 10,775 55
output 14.68 MB 2.27 MB
* with the matching @sistent/mui-datatables deep-import fix
(layer5io/mui-datatables), which removes the remaining barrel.
The build succeeds (ESM, CJS and declarations), all 515 tests pass, and lint
is clean. The public API is unchanged.
Not changed: `noExternal: [/^@meshery\/schemas/]` (c34bfaa) is what grew the
package from 2.3 MB in 0.18.0 to 9.3 MB in 0.18.1, but it was added to fix a
schema version mismatch in the Meshery UI, so un-bundling it needs a
maintainer decision rather than a drive-by change.
Signed-off-by: saurabhraghuvanshii <saurabhsraghuvanshi@gmail.com>
saurabhraghuvanshii
force-pushed
the
perf/tree-shakeable-package
branch
from
September 13, 2026 22:06
8b46a58 to
d492352
Compare
banana-three-join
approved these changes
Sep 18, 2026
Pointing `module` at `dist/index.mjs` exposed three ways the ESM build had
never actually been loaded by Node. Bundlers tolerate all three, so nothing
caught them; Next.js SSR does not, and Meshery failed with "Failed to collect
page data".
- `import { startCase } from 'lodash'` and two `import { debounce }` —
lodash is CommonJS, and Node's ESM loader cannot detect its named exports:
"Named export 'startCase' not found". Now imported as
`lodash/startCase.js` and `lodash/debounce.js`, which also tree-shakes.
- `use-sync-external-store` (pulled in by @xstate/react, react-redux and
@mui/x-date-pickers) is CommonJS and calls `require('react')` at runtime.
Bundled into ESM output, esbuild can only emit that as a `__require` shim,
which throws `Dynamic require of "react" is not supported`. It is now
external, and declared as a dependency so consumers always resolve it.
- Dropped the `exports` map added in the previous commit. `main` + `module`
already give bundlers the ESM build and Node the CJS one, and the map adds
a resolution failure mode for no benefit here.
Verified: `import('@sistent/sistent')` now succeeds under Node 22 (735
exports), and a full Meshery UI production build (Next.js 16, Turbopack)
completes with zero errors, including
`/configuration/designs/configurator`. Build, lint and all 515 tests pass.
Consumer measurement is unchanged from the previous commit — webpack 5, a
consumer importing only `Box`, with the matching @sistent/mui-datatables fix:
14,132 -> 5,312 modules, 10,775 -> 55 icon modules, 14.68 MB -> 2.27 MB.
Signed-off-by: saurabhraghuvanshii <saurabhsraghuvanshi@gmail.com>
banana-three-join
requested changes
Sep 23, 2026
banana-three-join
left a comment
banana-three-join
left a comment
Contributor
There was a problem hiding this comment.
Build issues in Meshery and Cloud and being investigated...
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consumers could not tree-shake @sistent/sistent at all. Importing one component from 0.22.6 pulls ~14,000 modules and 14.7 MB into a webpack build, 10,775 of those modules being @mui/icons-material. In layer5.io this was the largest single contributor to build memory.
Three causes, all fixed here:
modulepointed at the CommonJS build.mainandmodulewere both./dist/index.js, so no bundler ever selecteddist/index.mjs.modulenow points at the ESM build, and anexportsmap routesimportandrequireto the matching build and declarations.There was no
sideEffectsfield, so bundlers had to keep every export. The package has no top-level side effects (no CSS imports, no global registration), so it is now markedsideEffects: false.Six components imported icons from the
@mui/icons-materialbarrel, which re-exports ~10,800 modules. They now import by path, and ano-restricted-importsrule keeps the barrel out.Serving the ESM build exposed two imports that only worked through CJS interop, and would have broken strict ESM consumers:
import jsyaml from 'js-yaml'has no default export in js-yaml's ESM build; nowimport * as jsyaml.lodash/debouncehas no extension and lodash has no exports map, which webpack rejects from an.mjsfile (fullySpecified); nowlodash/debounce.js.Measured with webpack 5, a consumer that imports only
Box:published 0.22.6 this change*
modules 14,132 5,288
@mui/icons-material 10,775 55
output 14.68 MB 2.27 MB
The build succeeds (ESM, CJS and declarations), all 515 tests pass, and lint is clean. The public API is unchanged.
Not changed:
noExternal: [/^@meshery\/schemas/](c34bfaa) is what grew the package from 2.3 MB in 0.18.0 to 9.3 MB in 0.18.1, but it was added to fix a schema version mismatch in the Meshery UI, so un-bundling it needs a maintainer decision rather than a drive-by change.Notes for Reviewers
This PR fixes #
Signed commits
Summary by CodeRabbit
Compatibility
Maintenance