-
Notifications
You must be signed in to change notification settings - Fork 278
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
build: update vite version and fix inline-chunk plugin #2722
Conversation
WalkthroughThis pull request involves modifications to 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 (
|
概要此PR更新了构建脚本中Vite的版本,并修复了内联功能依赖插件不生效的问题。主要修改包括正则表达式的调整以匹配新的文件命名格式。 变更
|
@@ -27,7 +27,7 @@ export default function ({ deleteInlinedFiles = true }): Plugin { | |||
if (!jsChunk.code) continue | |||
|
|||
if (format === 'es') { | |||
const reg = /^import(\s*.+\s*from)?\s+"[./]+(.+-[a-f0-9]{8}.+)".*$/gim | |||
const reg = /^import(\s*.+\s*from)?\s+"[./]+(.+-[A-Za-z0-9_-]{8}\.[mc]?js)".*$/gim |
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.
The regular expression has been updated to match a broader range of file name patterns, including different extensions and character sets. Ensure that this change does not inadvertently match unintended file names.
@@ -40,7 +40,7 @@ export default function ({ deleteInlinedFiles = true }): Plugin { | |||
} | |||
|
|||
if (format === 'cjs') { | |||
const reg = /require\("[./]+(.+-[a-f0-9]{8}.+)".*$/gim | |||
const reg = /require\("[./]+(.+-[A-Za-z0-9_-]{8}\.[mc]?js)".*$/gim |
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.
The regular expression for 'require' statements has been updated similarly to the 'import' statements. Verify that this change aligns with the intended file patterns and does not introduce any security vulnerabilities.
SummaryThis PR updates the version of Vite in the build script and fixes the problem that inline function dependent plug-ins do not take effect. Major modifications include adjustments to regular expressions to match the new file naming format. Change
|
[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: 0
🧹 Nitpick comments (2)
internals/cli/src/commands/build/rollup/inline-chunks.ts (2)
30-30
: Consider supporting single quotes or dynamic imports
Current regex only matches double-quoted static imports. You might wish to handle single quotes ('./path'
) or dynamic imports (import('./path')
) for broader compatibility.
43-43
: Consider handling single quotes in CommonJS requires
Similar to the ES import pattern,require('./path')
can also be written with single quotes. Updating the regex will ensure consistent handling of different quoting styles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internals/cli/package.json
(2 hunks)internals/cli/src/commands/build/rollup/inline-chunks.ts
(2 hunks)
🔇 Additional comments (5)
internals/cli/package.json (5)
5-5
: No immediate concerns with enabling "private"
Keeping the CLI project private is consistent with internal usage and distribution.
7-7
: "main" field looks good
Pointing to"./dist/cjs/index.js"
aligns with standard Node packaging conventions.
36-36
: Dependency addition
Adding"@opentiny/huicharts": "~1.0.0"
is fine; confirm the tilde range aligns with your version-management expectations.
44-44
: No objections to "vite-svg-loader"
Upgrading or retaining"vite-svg-loader": "^3.6.0"
is standard. No issues observed.
46-67
: Verify necessity of devDependencies
A large devDependencies set can increase maintenance overhead. Please confirm that each entry is required and up to date.✅ Verification successful
All devDependencies are actively used and necessary
Based on the codebase analysis, all devDependencies in
internals/cli/package.json
are actively used:
Build tools:
@babel/*
: Used for code transpilation in build scriptsrollup
,tsup
,vite
: Used as build tools in various build commandstypescript
: Used for TypeScript compilation throughout the codebaseCLI and utilities:
commander
: Used for CLI command handling insrc/index.ts
chalk
: Used for console output styling in multiple filesfs-extra
: Used extensively for file operationsfind-up
: Used in build scripts for path resolutionhandlebars
: Used for template rendering in build scriptsprettier
: Used for code formattingsemver
: Used for version handling in multiple placesVue/Template related:
@opentiny-internal/unplugin-virtual-template
: Used in build configurations@opentiny/vue-vite-template2jsx
: Used for JSX template handling@vue/tsconfig
: Extended in TypeScript configurationsBundler plugins:
@rollup/plugin-babel
: Used in build configurations@rollup/plugin-commonjs
: Used in runtime buildsThe dependencies have appropriate versions and are actively used in the codebase. No unnecessary dependencies were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Script to locate references to devDependencies in the codebase # and confirm if they are actually in use. # 1. List all devDependencies devDependencies=( "@babel/core" "@babel/preset-env" "@opentiny-internal/unplugin-virtual-template" "@opentiny/vue-vite-template2jsx" "@rollup/plugin-babel" "@rollup/plugin-commonjs" "@types/fs-extra" "@vue/tsconfig" "chalk" "commander" "esbuild-register" "find-up" "fs-extra" "handlebars" "prettier" "rollup" "semver" "tailwindcss" "tsup" "typescript" "vite" ) # 2. Search for each devDependency usage across the repository for dep in "${devDependencies[@]}"; do echo "Searching usage of $dep..." rg "$dep" -A 5 doneLength of output: 69601
PR
升级构建脚本的vite版本以及修复内联功能依赖插件不生效问题
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
Chores
package.json
for internal CLI project@opentiny/huicharts
dependencyvite-svg-loader
Refactor