feat(compiler-sfc): add preserveLeadingTilde option for asset URL and srcset transformations
#13462
Conversation
WalkthroughAdded an optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Compiler as TemplateCompiler
participant Asset as transformAssetUrl
participant Src as transformSrcset
participant Parse as parseUrl
Note over Compiler,Asset: Asset URL transform (optionally preserve leading ~)
Compiler->>Asset: compile(template, options)
Asset->>Parse: parseUrl(attr.value.content, options.preserveLeadingTilde)
Parse-->>Asset: parsed URL (leading ~ preserved if flag true)
Asset-->>Compiler: transformed template
Note over Compiler,Src: Srcset transform (optionally preserve leading ~)
Compiler->>Src: compileSrcset(template, options)
Src->>Parse: parseUrl(srcsetUrl, options.preserveLeadingTilde)
Parse-->>Src: parsed srcset URL (leading ~ preserved if flag true)
Src-->>Compiler: transformed srcset
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Comment |
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 |
edison1105
changed the title
preserveLeadingTilde option for asset URL and srcset transformations
close #13460
close vitejs/vite-plugin-vue#15
This PR introduces a new option,
preserveLeadingTilde, 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