-
Notifications
You must be signed in to change notification settings - Fork 276
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
fix(renderless): fix renderless package publish error #2761
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
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 (
|
WalkthroughThis pull request addresses an issue with the renderless package publish process by modifying the Changes
|
@@ -4,6 +4,7 @@ import { promises as fs } from 'node:fs' | |||
async function run() { | |||
const content = await fs.readFile(resolve('package.json'), 'utf8') | |||
const packageJson = JSON.parse(content) | |||
packageJson.dependencies['@opentiny/utils'] = '^1.0.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that adding the @opentiny/utils
dependency with a fixed version (^1.0.0
) does not conflict with other dependencies or cause versioning issues in the future.
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/renderless/scripts/postbuild.ts (1)
Line range hint
12-12
: Improve script operational visibilityAdd logging to track the script's progress and success/failure status.
async function run() { + console.log('Starting postbuild script...') try { const content = await fs.readFile(resolve('package.json'), 'utf8') const packageJson = JSON.parse(content) if (!packageJson.dependencies) { packageJson.dependencies = {} } packageJson.dependencies['@opentiny/utils'] = '^1.0.0' delete packageJson.exports delete packageJson.private await fs.writeFile(resolve('dist', 'package.json'), JSON.stringify(packageJson, null, 2)) + console.log('Successfully updated dist/package.json') } catch (error) { console.error('Failed to process package.json:', error) process.exit(1) } }🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 9-9: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/renderless/scripts/postbuild.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Unit Test
🔇 Additional comments (3)
packages/renderless/scripts/postbuild.ts (3)
7-7
: Version constraint concerns for @opentiny/utilsUsing a caret version range (^1.0.0) might lead to compatibility issues if breaking changes are introduced in minor versions.
Consider:
- Using a more restrictive version range
- Documenting the version requirement rationale
- Adding a version validation check
7-7
: 🛠️ Refactor suggestionEnsure dependencies object exists before modification
The script assumes the existence of the dependencies object. Add a safety check to prevent runtime errors.
+ if (!packageJson.dependencies) { + packageJson.dependencies = {} + } packageJson.dependencies['@opentiny/utils'] = '^1.0.0'Likely invalid or redundant comment.
7-10
: Verify build process compatibilityLet's ensure this change doesn't affect other packages or create circular dependencies.
✅ Verification successful
Dependency addition is safe and appropriate
The addition of
@opentiny/utils
dependency is consistent with the actual usage in the renderless package, with no circular dependencies detected. The change properly formalizes an existing dependency relationship.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential circular dependencies and usage echo "Checking for circular dependencies..." rg -l "@opentiny/utils" packages/renderless/ rg -l "renderless" packages/utils/ echo "Checking package.json in other packages..." find packages -name "package.json" -not -path "*/node_modules/*" -exec sh -c ' echo "Checking {}..." cat {} | jq -r ".dependencies | select(.\"@opentiny/utils\" != null) | .\"@opentiny/utils\"" ' \;Length of output: 16701
🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 9-9: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
async function run() { | ||
const content = await fs.readFile(resolve('package.json'), 'utf8') | ||
const packageJson = JSON.parse(content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for file operations
The file operations lack try-catch blocks which could lead to silent failures during the build process. Consider adding proper error handling and validation.
async function run() {
+ try {
const content = await fs.readFile(resolve('package.json'), 'utf8')
const packageJson = JSON.parse(content)
packageJson.dependencies['@opentiny/utils'] = '^1.0.0'
delete packageJson.exports
delete packageJson.private
await fs.writeFile(resolve('dist', 'package.json'), JSON.stringify(packageJson, null, 2))
+ } catch (error) {
+ console.error('Failed to process package.json:', error)
+ process.exit(1)
+ }
}
Also applies to: 10-10
PR
修复renderless包发布时候无法正常发布的问题
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
@opentiny/utils
package to the build process.