feat: shared build #19
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces significant changes to the build configuration and module export strategy for the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/shared/package.json (2)
10-10: Optimize build scripts to prevent redundant builds.The
prepackscript runs the same commands as thebuildscript, which is already called byprepare. This could lead to redundant builds.Consider this optimization:
- "prepack": "npm run build:esm && npm run build:cjs", - "prepare": "npm run build", + "prepack": "npm run build", + "prepare": "npm run build"Also applies to: 11-11
15-16: Consider conditional exports for browser field.The browser field points to ESM modules, but some bundlers might prefer CJS. Consider using conditional exports instead.
Example approach:
- "browser": { - "./types": "./dist/esm/types/index.js", - "./utils": "./dist/esm/utils/index.js" - }, + "exports": { + "./types": { + "browser": "./dist/esm/types/index.js", + "import": "./dist/esm/types/index.js", + "require": "./dist/cjs/types/index.js" + }, + "./utils": { + "browser": "./dist/esm/utils/index.js", + "import": "./dist/esm/utils/index.js", + "require": "./dist/cjs/utils/index.js" + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/native/index.js(4 hunks)packages/shared/package.json(1 hunks)packages/shared/tsconfig.cjs.json(1 hunks)packages/shared/tsconfig.json(1 hunks)packages/shared/tsup.config.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/shared/tsup.config.ts
✅ Files skipped from review due to trivial changes (2)
- packages/shared/tsconfig.cjs.json
- packages/native/index.js
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: eslint_test
🔇 Additional comments (5)
packages/shared/tsconfig.json (3)
12-12: LGTM! Good practice for source organization.Narrowing the include paths to
src/**/*is a good practice for better source organization and build clarity.
8-9: Consider future Node.js version compatibility.Using
"Node16"for bothmoduleandmoduleResolutionties the package to Node.js 16+. This is a relatively recent version that might not be supported in all environments.Let's check the Node.js version requirements in the project:
4-9: Verify browser compatibility with ES2018 target.The target has been updated to ES2018. While this provides access to newer JavaScript features, ensure this aligns with your browser support requirements.
Run this command to check browser compatibility:
✅ Verification successful
ES2018 target is compatible with browser requirements
The ES2018 target in
packages/shared/tsconfig.jsonis compatible with the browserslist configurations found in the apps. Both React and Vue apps target modern browsers that fully support ES2018 features.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check browser compatibility for ES2018 features # Look for any browserslist config in the project fd -t f -e json -e js -e yml -e yaml -e rc '(package|browserslist|babel)' | xargs cat | grep -i browserslistLength of output: 150
Script:
#!/bin/bash # Get complete browserslist configuration echo "=== Package.json browserslist ===" fd -t f "package.json" -x sh -c 'echo "File: {}"; cat {} | jq -r ".browserslist"' echo -e "\n=== Standalone browserslist config ===" fd -t f "^\.browserslistrc$" -x sh -c 'echo "File: {}"; cat {}'Length of output: 1533
packages/shared/package.json (2)
25-28: LGTM! Explicit files inclusion.The
filesfield explicitly includes the necessary directories for distribution.
19-23: Verify the wildcard exports configuration.The new exports configuration uses wildcards for both ESM and CJS paths. While this provides flexibility, ensure all necessary files are properly built and exported.
Let's verify the export paths:
✅ Verification successful
The exports configuration is correctly structured ✅
The wildcard exports configuration is properly aligned with:
- The source files structure in
src/directory- The build scripts that generate both ESM and CJS outputs
- The
filesfield that ensures all necessary directories are included in the package🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if all source files have corresponding build outputs echo "Checking source files..." fd -t f . src/ -e ts -x echo "Source: {}" \ -x sh -c 'echo "ESM: dist/esm/${0#src/}" && echo "CJS: dist/cjs/${0#src/}"' {} \;Length of output: 279
Script:
#!/bin/bash # Find source files echo "=== Source Files ===" fd -e ts -e js echo -e "\n=== Directory Structure ===" ls -la echo -e "\n=== Package JSON Content ===" cat packages/shared/package.json echo -e "\n=== Build Output Structure ===" ls -la packages/shared/dist 2>/dev/null || echo "dist directory not found"Length of output: 4456
| "build:esm": "tsc -p tsconfig.json && echo '{\"type\": \"module\"}' > dist/esm/package.json", | ||
| "build:cjs": "tsc -p tsconfig.cjs.json && echo '{\"type\": \"commonjs\"}' > dist/cjs/package.json", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve cross-platform compatibility of build scripts.
The echo commands for creating package.json files might not work correctly on Windows. Consider using a cross-platform solution.
Here's a more robust approach:
- "build:esm": "tsc -p tsconfig.json && echo '{\"type\": \"module\"}' > dist/esm/package.json",
- "build:cjs": "tsc -p tsconfig.cjs.json && echo '{\"type\": \"commonjs\"}' > dist/cjs/package.json",
+ "build:esm": "tsc -p tsconfig.json && node -e \"fs.writeFileSync('dist/esm/package.json', JSON.stringify({type:'module'}, null, 2))\"",
+ "build:cjs": "tsc -p tsconfig.cjs.json && node -e \"fs.writeFileSync('dist/cjs/package.json', JSON.stringify({type:'commonjs'}, null, 2))\""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "build:esm": "tsc -p tsconfig.json && echo '{\"type\": \"module\"}' > dist/esm/package.json", | |
| "build:cjs": "tsc -p tsconfig.cjs.json && echo '{\"type\": \"commonjs\"}' > dist/cjs/package.json", | |
| "build:esm": "tsc -p tsconfig.json && node -e \"fs.writeFileSync('dist/esm/package.json', JSON.stringify({type:'module'}, null, 2))\"", | |
| "build:cjs": "tsc -p tsconfig.cjs.json && node -e \"fs.writeFileSync('dist/cjs/package.json', JSON.stringify({type:'commonjs'}, null, 2))\"" |
Summary by CodeRabbit
Release Notes
Build Configuration
Development
Package Structure