feat(compiler-sfc): add preserveTilde option for asset URL and srcset transformations #13462
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant TemplateCompiler
participant TransformAssetUrl
participant TransformSrcset
participant ParseUrl
TemplateCompiler->>TransformAssetUrl: compile(template, { preserveTilde })
TransformAssetUrl->>ParseUrl: parseUrl(url, preserveTilde)
ParseUrl-->>TransformAssetUrl: returns parsed URL (tilde preserved if option true)
TransformAssetUrl-->>TemplateCompiler: transformed asset URL
TemplateCompiler->>TransformSrcset: compileSrcset(template, { preserveTilde })
TransformSrcset->>ParseUrl: parseUrl(srcsetUrl, preserveTilde)
ParseUrl-->>TransformSrcset: returns parsed srcset URL (tilde preserved if option true)
TransformSrcset-->>TemplateCompiler: transformed srcset
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(No out-of-scope changes found.) Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
edison1105
added
✨ feature request
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/compiler-sfc/src/template/templateUtils.ts (1)
22-31:preserveTildeflag integrates cleanly – consider future-proofing with WHATWG URLThe early-return logic is solid and preserves existing behaviour when the flag is falsy ✅.
However,url.parse(the legacy API behinduriParse) is deprecated in recent Node versions; the WHATWGURLconstructor is now the recommended API. Migrating at some point would avoid deprecation warnings and give stricter parsing.No action required for this PR, but adding a tech-debt ticket now will save churn later.
packages/compiler-sfc/src/template/transformAssetUrl.ts (1)
123-129: Redundant parse onoptions.base
parseUrlis executed twice in the same branch – once forattr.value.contentand again foroptions.base. The second invocation is unavoidable, but you could memoise it locally to avoid reparsingbasefor every attribute.-const base = parseUrl(options.base, options.preserveTilde) +const baseUrl = parseUrl(options.base, options.preserveTilde)Minor perf win, feel free to ignore.
packages/compiler-sfc/__tests__/templateTransformAssetUrl.spec.ts (1)
103-111: 👍 Test covers both~/and~alias formsSnapshot ensures both syntaxes compile. Consider asserting the generated import path explicitly (e.g. contains
import _imports_0 from '~/app/bar.png') to make the test more intention-revealing.packages/compiler-sfc/__tests__/templateTransformSrcset.spec.ts (1)
102-111: Good parity with asset-url testsMirrors the asset-url testcase; same note about being explicit applies here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/compiler-sfc/__tests__/__snapshots__/templateTransformAssetUrl.spec.ts.snapis excluded by!**/*.snappackages/compiler-sfc/__tests__/__snapshots__/templateTransformSrcset.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
packages/compiler-sfc/__tests__/templateTransformAssetUrl.spec.ts(1 hunks)packages/compiler-sfc/__tests__/templateTransformSrcset.spec.ts(1 hunks)packages/compiler-sfc/src/template/templateUtils.ts(1 hunks)packages/compiler-sfc/src/template/transformAssetUrl.ts(2 hunks)packages/compiler-sfc/src/template/transformSrcset.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/compiler-sfc/src/template/transformSrcset.ts (1)
packages/compiler-sfc/src/template/templateUtils.ts (1)
parseUrl(22-32)
packages/compiler-sfc/src/template/transformAssetUrl.ts (1)
packages/compiler-sfc/src/template/templateUtils.ts (1)
parseUrl(22-32)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: test / unit-test-windows
🔇 Additional comments (3)
packages/compiler-sfc/src/template/transformAssetUrl.ts (2)
35-41: Option contract extended correctly – please bubble the docs
preserveTildeis added with a clear JSDoc and default. Looks good.
Remember to surface the new option in user-facing docs / typings packages (@vue/compiler-sfc).
46-47: Default updated – snapshot tests already cover itNo issues spotted.
packages/compiler-sfc/src/template/transformSrcset.ts (1)
111-112:preserveTildecorrectly forwardedPassing
options.preserveTildekeeps behaviour consistent withtransformAssetUrl. Good catch.
|
this looks great! thank you ❤️ is it still possible to import from modules, ie (I would imagine that would just be up to the bundler to resolve, and would therefore work with vite.) |
|
You're absolutely right! With |
close #13460
close vitejs/vite-plugin-vue#15
This PR introduces a new option,
preserveTilde, for handling the~in asset URLs.false, which keeps the existing behavior: leading~and~/are stripped - it's exactly the same as before.true, leading~and~/are preserved and resolution relies on the bundler (e.g., Vite).Usage example:
Summary by CodeRabbit