fix(vite-config): banner-aware 'use client' directive detection - #3
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
The new comment-stripping logic can still miss directives when chunks start with mixed // and /* */ comments in varying order, allowing duplicates to persist in some cases.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes @eventuras/vite-config’s preserve-use-client Vite plugin to avoid prepending a duplicate 'use client' directive when Rollup output chunks begin with banners/license comments.
Changes:
- Strip leading banner/license comments before checking whether a chunk already starts with
'use client'. - Add a changeset to publish a patch release of
@eventuras/vite-config.
File summaries
| File | Description |
|---|---|
| config/vite-config/src/react-lib.ts | Updates directive-detection logic to skip leading comments when deciding whether to prepend 'use client'. |
| .changeset/banner-aware-use-client.md | Adds a patch changeset documenting the behavior fix. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| const codeStart = chunkData.code | ||
| .replace(/^\s+/, '') | ||
| .replace(/^(\/\*[\s\S]*?\*\/\s*)+/, '') | ||
| .replace(/^(\/\/.*(\r?\n|$)\s*)+/, ''); |
The preserve-use-client plugin checked only code.trimStart() for an existing directive, so chunks that begin with a banner comment (e.g. Rollup's output.banner, SPDX license headers) always failed the check and got a duplicate 'use client' prepended above the banner. Strip leading comments via a single shared helper that handles any interleaved mix of line and block comments — sequential per-style passes miss mixed orders. The same helper also replaces the weaker stripping in the source-module scan (hasClientDirective), which missed block-after-line orders and consecutive block comments, silently dropping the directive from output. Ported from ratio-ui, which carries the banner fix in its forked copy of the config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5a06715 to
bb55e86
Compare
|
Applied the suggested single-pass strip — sequential per-style passes do miss interleaved orders (verified: Also extracted it as a shared |
Summary
The
preserve-use-clientplugin in@eventuras/vite-configchecks whether a chunk already starts with a'use client'directive before prepending one. That check usedcode.trimStart(), so any chunk beginning with a comment — e.g. a license header injected via Rollup'soutput.banner— always failed the check and got a duplicate directive prepended above the banner.This ports the fix ratio-ui carries in its forked copy of this config: strip leading block and line comments before checking for the directive.
Context
ratio-ui builds all packages with SPDX license banners, and is about to drop its forked
config/packages in favor of the published@eventuras/*ones — this fix is the one thing its fork had that upstream lacked.Changes
config/vite-config/src/react-lib.ts: skip leading/* … */and// …comments before the existing-directive check@eventuras/vite-config🤖 Generated with Claude Code