refactor: replace @vue/compiler-sfc parsing
#3848
Conversation
WalkthroughThe changes implement a significant architectural shift in Vue SFC processing and macro transformation. The Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (8)
src/transform/macros.ts (3)
17-17: Avoid dynamic RegExp; use a static literal.DEFINE_I18N_ROUTE_FN is constant, so build a static regex (or a simple string include) to avoid needless dynamic construction and to appease linters. Example: const I18N_MACRO_FN_RE = /\bdefineI18nRoute\s*(\s*/.
49-55: Skip shadowed identifiers using ScopeTracker.Currently any local function named defineI18nRoute is transformed. Guard with ScopeTracker to avoid false positives.
const name = node.callee.name if (name !== DEFINE_I18N_ROUTE_FN) return + // Ignore locally shadowed identifiers + if (scopeTracker.isDeclared?.(name)) return s.overwrite(node.start, node.end, ` false && /*@__PURE__*/ ${DEFINE_I18N_ROUTE_FN}${code.slice(node.callee.end, node.end)}`)Docs show ScopeTracker helpers (isDeclared, getDeclaration). (socket.dev)
58-60: Improve error context.Prefix errors with plugin name and file id for debuggability.
- console.error(e) + console.error('[nuxtjs:i18n-macros-transform]', id, e)src/utils/extract-script.ts (2)
7-7: Make loader detection explicit via lang= to reduce false positives.Rely on lang= when present; fall back to jsx/tsx hint; default to 'ts'.
- loader: match.groups.attrs && /[tj]sx/.test(match.groups.attrs) ? 'tsx' : 'ts', + loader: (() => { + const attrs = match.groups.attrs ?? '' + const lang = (attrs.match(/\blang\s*=\s*["']?([a-z0-9_-]+)/i)?.[1] || '').toLowerCase() + if (lang === 'tsx' || /\bjsx\b/i.test(attrs)) return 'tsx' + return 'ts' + })(),
1-11: Document/decide behavior for<script src="...">blocks.Scripts with external src will be skipped (empty innerHTML). If unsupported by design, note this in docs; otherwise, add resolution logic.
src/pages.ts (3)
394-429: Ignore shadoweddefineI18nRoutevia ScopeTracker.Avoid capturing user-defined functions named
defineI18nRoute. Add a ScopeTracker and skip when locally declared.- for (const script of blocks) { + for (const script of blocks) { if (extract != null) break - parseAndWalk(script.code, absolutePath.replace(/\.\w+$/, '.' + script.loader), (node) => { + const scopeTracker = new ScopeTracker({ preserveExitedScopes: true }) + parseAndWalk( + script.code, + absolutePath.replace(/\.\w+$/, '.' + script.loader), + { + scopeTracker, + enter(node) { if (extract != null) return let code = script.code if ( node.type !== 'CallExpression' || node.callee.type !== 'Identifier' || node.callee.name !== DEFINE_I18N_ROUTE_FN ) return + // Skip if symbol is shadowed in the current scope + if (scopeTracker.isDeclared?.(DEFINE_I18N_ROUTE_FN)) return let routeArgument = node.arguments[0] if (routeArgument == null) return - if (typeof script.loader === 'string' && /tsx?/.test(script.loader)) { + if (typeof script.loader === 'string' && /tsx?/.test(script.loader)) { const transformed = transform('', script.code.slice(node.start, node.end).trim(), { lang: script.loader }) code = transformed.code if (transformed.errors.length) { for (const error of transformed.errors) { console.warn(`Error while transforming \`${DEFINE_I18N_ROUTE_FN}()\`` + error.codeframe) } return } // we already know that the first statement is a call expression routeArgument = ( (parseSync('', transformed.code, { lang: 'js' }).program.body[0]! as ExpressionStatement) .expression as CallExpression ).arguments[0]! as ObjectExpression } extract = evalAndValidateValue(code.slice(routeArgument.start, routeArgument.end).trim()) - }) + } + ) }Docs show
parseAndWalk(..., { scopeTracker, enter })and helpers likeisDeclared. (socket.dev)
414-426: Follow-up: keep parseSync filename meaningful.Optional: set a synthetic filename for
parseSynctoo (e.g.,inline.js) to aid diagnostics and align with parser heuristics.- (parseSync('', transformed.code, { lang: 'js' }).program.body[0]! as ExpressionStatement) + (parseSync('inline.js', transformed.code, { lang: 'js' }).program.body[0]! as ExpressionStatement)
388-391: Sanity check: prefilter with includes().Good fast-path. Consider a slightly stricter prefilter like
/\bdefineI18nRoute\s*\(/to reduce false positives from comments/template.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(0 hunks)src/pages.ts(2 hunks)src/transform/macros.ts(2 hunks)src/utils/extract-script.ts(1 hunks)
💤 Files with no reviewable changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages.ts (3)
src/utils/extract-script.ts (1)
extractScriptContent(2-14)src/constants.ts (1)
DEFINE_I18N_ROUTE_FN(88-88)src/transform/resource.ts (1)
transform(14-17)
src/transform/macros.ts (2)
src/constants.ts (1)
DEFINE_I18N_ROUTE_FN(88-88)src/transform/utils.ts (1)
BundlerPluginOptions(45-47)
🪛 ast-grep (0.39.6)
src/transform/macros.ts
[warning] 16-16: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${DEFINE_I18N_ROUTE_FN}\\s*\\(\\s*)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (3)
src/transform/macros.ts (1)
23-23: Confirm plugin ordering after switching to enforce: 'post'.Changing to post may alter interaction with other transforms (e.g., Vue SFC, typed-router). Please verify this still runs after SFC splitting but before minification/treeshake in your target bundlers.
src/pages.ts (2)
392-399: Nice: multi-block parsing + early exit.Iterating over all script blocks and exiting on first hit simplifies flow and avoids descriptor coupling.
394-429: No shadowing or external scripts detected—verification concerns unfounded.The search results confirm:
No shadowed
defineI18nRoutedeclarations: All usages import from'#i18n'macro (e.g.,import { defineI18nRoute } from '#i18n'). No local redeclarations exist in any Vue files across the codebase.No external
<script src>tags: Zero matches for<script ... src=patterns in Vue files. All script blocks are inline, not external references.The verification scenarios outlined in the review comment do not occur in practice, so the code at
src/pages.tslines 394-429 handles the actual cases it will encounter correctly.
🔗 Linked issue
📚 Description
WIP
This is to remove vue compiler sfc as direct prod dependency
Summary by CodeRabbit
Refactor
Chores