CLOUDP-352308 Publish both non-minified and minified bundles for dev and prod environments #3316
Conversation
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
|
nima-taheri-mongodb
force-pushed
the
cloudp-352308-publish-both-dev-prod
branch
from
4ac1d84 to
5aba0c7
Compare
Contributor
|
Size Change: +576 kB (+29.53%) 🚨 Total Size: 2.52 MB
ℹ️ View Unchanged
|
nima-taheri-mongodb
changed the title
nima-taheri-mongodb
changed the title
Collaborator
Author
|
super curious why this weird import pops up, added a few more commits to collect troubleshooting data.. |
nima-taheri-mongodb
requested review from
Copilot and
stephl3
and removed request for
a team
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates the build process to generate both non-minified and minified JavaScript bundles, improving debugging capabilities while still providing optimized production builds. The non-minified bundle becomes the default export, with minified versions available via production-specific package.json exports.
Key Changes:
- Added a new
minifybuild step using Terser to create minified bundles (*-min.js) from non-minified outputs - Updated package.json exports across all packages to provide minified bundles via the
productioncondition - Modified Rollup configuration to remove inline minification and generate non-minified bundles by default
Reviewed Changes
Copilot reviewed 108 out of 109 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Adds minify task to build pipeline with proper dependencies and outputs |
| tools/build/src/minify.ts | New minification utility using Terser to generate minified bundles with source maps |
| tools/build/src/cli.ts | Adds lg-build minify command with glob pattern options |
| tools/build/config/rollup.config.mjs | Removes Rollup terser plugin, changes to external source maps, adds visualizer plugin |
| tools/build/config/babel.config.js | Removes production-specific module handling |
| tools/build/package.json | Replaces @rollup/plugin-terser with terser dependency |
| packages/*/package.json | Updates exports to include production-specific minified bundle paths |
| chat/*/package.json | Updates exports to include production-specific minified bundle paths |
| charts/*/package.json | Updates exports to include production-specific minified bundle paths |
| package.json | Adds build:minify script to run minification across all packages |
| .github/workflows/release.yml | Adds minification step before npm publishing |
| .github/workflows/pr.yml | Adds artifact upload for built packages |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| configFile: babelConfigPath, | ||
| sourceMaps: 'inline', | ||
| envName: 'production', | ||
| sourceMaps: true, |
There was a problem hiding this comment.
The sourceMaps option should be sourceMap (singular) according to Babel's documentation. While sourceMaps may work due to backwards compatibility, using the correct option name ensures forward compatibility.
Suggested change
| sourceMaps: true, | |
| sourceMap: true, |
Comment on lines
+24
to
+25
| "import": "./dist/esm/*.js-min.js", | ||
| "require": "./dist/umd/*.js-min.js" |
There was a problem hiding this comment.
The minified file paths have incorrect double extensions *.js-min.js instead of *-min.js. This pattern will not match any generated files since the minify script creates files with the pattern baseName-min.js.
Comment on lines
+55
to
+56
| fs.writeFileSync(minifiedFile, minified.code!); | ||
| fs.writeFileSync(minifiedFileMap, minified.map! as string); |
There was a problem hiding this comment.
If minified.code or minified.map is undefined, the non-null assertion will cause a runtime error. Consider adding explicit error handling with a descriptive message before writing files.
Suggested change
| fs.writeFileSync(minifiedFile, minified.code!); | |
| fs.writeFileSync(minifiedFileMap, minified.map! as string); | |
| if (minified.code === undefined) { | |
| throw new Error(`Minification failed: 'code' is undefined for file ${jsFile}`); | |
| } | |
| if (minified.map === undefined) { | |
| throw new Error(`Minification failed: 'map' is undefined for file ${jsFile}`); | |
| } | |
| fs.writeFileSync(minifiedFile, minified.code); | |
| fs.writeFileSync(minifiedFileMap, minified.map as string); |
| "scripts": { | ||
| "prebuild": "ts-node ./scripts/prebuild/index.ts", | ||
| "build": "ts-node ./scripts/build/build.ts", | ||
| "build": "ts-node ./scripts/build/build.ts --verbose", |
There was a problem hiding this comment.
[nitpick] The --verbose flag was added to the build script. If this is for debugging purposes, consider whether it should remain in the committed code or if verbosity should be controlled via an environment variable.
Suggested change
| "build": "ts-node ./scripts/build/build.ts --verbose", | |
| "build": "ts-node ./scripts/build/build.ts", |
| options?: BuildTypescriptOptions, | ||
| ) { | ||
| const { verbose } = options ?? { verbose: false }; | ||
| const { verbose, downlevel } = options ?? {}; |
There was a problem hiding this comment.
The destructured update option is defined in the interface but not extracted here, meaning it cannot be used in the function body. Either add update to the destructuring or remove it from the interface if unused.
nima-taheri-mongodb
force-pushed
the
cloudp-352308-publish-both-dev-prod
branch
9 times, most recently
from
ac86d28 to
0a23318
Compare
nima-taheri-mongodb
force-pushed
the
cloudp-352308-publish-both-dev-prod
branch
2 times, most recently
from
17c55d5 to
fdd87e0
Compare
Collaborator
Author
|
Hey folks, I'm going to use this branch to see if I can reproduce the flaky |
nima-taheri-mongodb
force-pushed
the
cloudp-352308-publish-both-dev-prod
branch
7 times, most recently
from
3f35a5d to
600d5b4
Compare
nima-taheri-mongodb
force-pushed
the
cloudp-352308-publish-both-dev-prod
branch
4 times, most recently
from
bac81ea to
78c5cb7
Compare
nima-taheri-mongodb
force-pushed
the
cloudp-352308-publish-both-dev-prod
branch
from
78c5cb7 to
a9e439a
Compare
Contributor
|
Coverage after merging cloudp-352308-publish-both-dev-prod into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🎫 Ticket
CLOUDP-352308
📝 Summary
This PR updates the build process so we generate both non-minified and minified versions of our bundles. The default export in
package.jsonis now the non-minified bundle, with the minified version provided as a production-specific export.Details
tools/build/src/minify.ts, accessed vialg-build minify, to minify all non-minified JavaScript files located indist.buildscript produces non-minified outputs, while a newbuild:minifyscript invokes theminifytask across all sub-projects (except those under@lg-tools/*) to generate minified bundles.package.jsonexportsto default to the non-minified bundle, adding the minified bundle as a dedicated production export.lg-build tscscript to correctly forward all received arguments.Rationale
🧪 Checklist
pnpm changesetand included documentation of changes