fix: allow arbitrary attributes on script tags without type errors#630
fix: allow arbitrary attributes on script tags without type errors#630
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Nuxt script input type was broadened: Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/types.ts`:
- Line 125: The NuxtUseScriptInput alias isn't wired into the public API—update
the function signatures and internal references that still use UseScriptInput so
the composable and utils accept the relaxed type: change the useScript(...)
parameter (the function named useScript) to accept NuxtUseScriptInput instead of
UseScriptInput and update the scriptInput variable/type in the utility functions
(the identifiers scriptInput in utils.ts) to NuxtUseScriptInput as well; ensure
any other export or wrapper that forwards to useScript is adjusted to propagate
NuxtUseScriptInput so attributes like 'tw-client-key' are allowed through the
public API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a628e45b-c19f-4e3c-a107-b510bc20df57
📒 Files selected for processing (1)
src/runtime/types.ts
Replace UseScriptInput with NuxtUseScriptInput in parameter types so arbitrary script attributes are accepted by the public API.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/composables/useScript.ts`:
- Around line 35-36: The partytown branch in useScript currently replaces the
script attrs with a new object literal ({ src, type: 'text/partytown' }) and
drops arbitrary/custom fields (e.g., tw-client-key, nonce); update that branch
inside useScript to merge and forward all existing attributes by combining the
original input and any options.partytown into the emitted object (preserve
input, include src and type: 'text/partytown' while spreading input and
options.partytown so custom attrs and nonce are retained). Ensure you reference
the existing variables input and options.partytown and produce the final object
by merging (input, options.partytown) with src/type enforced for Partytown.
- Around line 35-36: The type NuxtUseScriptInput has been defined so it narrows
away the string branch, which breaks existing useScript('...') calls even though
useScript normalizes string at runtime; update the NuxtUseScriptInput type to
explicitly preserve the string union (e.g., NuxtUseScriptInput = string |
(UseScriptInput & Record<string, unknown>)) so the overload/union keeps
bare-string calls while still accepting the object form—adjust the exported
NuxtUseScriptInput type and any related imports/exports to use this union
without changing the runtime normalization in useScript.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9e4f27a-91eb-4ef1-8448-f015c100e3b3
📒 Files selected for processing (2)
src/runtime/composables/useScript.tssrc/runtime/utils.ts
| export function useScript<T extends Record<symbol | string, any> = Record<symbol | string, any>>(input: NuxtUseScriptInput, options?: NuxtUseScriptOptions<T>): UseScriptContext<UseFunctionType<NuxtUseScriptOptions<T>, T>> { | ||
| input = typeof input === 'string' ? { src: input } : input |
There was a problem hiding this comment.
Forward custom attrs through the Partytown path.
After widening input, the options.partytown branch still emits only { src, type: 'text/partytown' }, so arbitrary attrs like tw-client-key—and existing fields like nonce—are silently dropped there. That leaves this fix incomplete for Partytown users.
Suggested change
- useHead({
- script: [{ src, type: 'text/partytown' }],
- })
+ useHead({
+ script: [{ ...input, src, type: 'text/partytown' }],
+ })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/composables/useScript.ts` around lines 35 - 36, The partytown
branch in useScript currently replaces the script attrs with a new object
literal ({ src, type: 'text/partytown' }) and drops arbitrary/custom fields
(e.g., tw-client-key, nonce); update that branch inside useScript to merge and
forward all existing attributes by combining the original input and any
options.partytown into the emitted object (preserve input, include src and type:
'text/partytown' while spreading input and options.partytown so custom attrs and
nonce are retained). Ensure you reference the existing variables input and
options.partytown and produce the final object by merging (input,
options.partytown) with src/type enforced for Partytown.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the current NuxtUseScriptInput definition
fd -t f "types.ts" src/runtime/ --exec cat -n {}Repository: nuxt/scripts
Length of output: 12564
🏁 Script executed:
# Check useScript.ts imports and the specific lines mentioned (35-36)
cat -n src/runtime/composables/useScript.ts | head -50Repository: nuxt/scripts
Length of output: 2632
🏁 Script executed:
# Find the git diff to see what changed in these files
git diff HEAD~1 src/runtime/types.ts src/runtime/composables/useScript.tsRepository: nuxt/scripts
Length of output: 244
🏁 Script executed:
# Search for useScript calls with string literals in test/example files
rg "useScript\s*\(\s*['\"]" --type ts --type js -A 2Repository: nuxt/scripts
Length of output: 7698
🏁 Script executed:
# Also check if there are any imports of UseScriptInput to understand its structure
rg "UseScriptInput" --type tsRepository: nuxt/scripts
Length of output: 1682
🏁 Script executed:
# Let's see what the `@unhead/vue/scripts` exports
# First, check if there are any node_modules or type definitions in the repo
fd -t f "node_modules/@unhead" -o -t f ".d.ts" | grep -i unhead | head -20Repository: nuxt/scripts
Length of output: 172
🏁 Script executed:
# Look for any documentation or README that might mention useScript('url')
rg "useScript\s*\(\s*['\"]https" --type ts --type js --type mdRepository: nuxt/scripts
Length of output: 4435
🏁 Script executed:
# Check package.json to see the `@unhead/vue` version
cat package.json | grep -A 5 '"@unhead/vue"'Repository: nuxt/scripts
Length of output: 374
🏁 Script executed:
# Search for any local type stubs or documentation about UseScriptInput
rg "UseScriptInput.*=" --type ts -B 2 -A 5Repository: nuxt/scripts
Length of output: 1237
🏁 Script executed:
# Check if there are any TypeScript compilation errors in tests
cat test/unit/check-scripts.test.ts | head -100Repository: nuxt/scripts
Length of output: 4017
🏁 Script executed:
# Let's verify if the string branch exists in UseScriptInput by checking unhead docs
web_searchRepository: nuxt/scripts
Length of output: 106
🌐 Web query:
@unhead/vue UseScriptInput type definition structure
💡 Result:
In @unhead/vue (v2.1.4), UseScriptInput is defined as:
type UseScriptInput =
| string
| (ResolvableProperties<
Omit<ScriptWithoutEvents & DataKeys & SchemaAugmentations['script'], 'src'>
> & { src: string })[1]
So you can pass either:
- A string → treated as the
srcURL (e.g.useScript('https://…/sdk.js')). [1] - An object with:
src: string(required)- any other
<script>fields from Unhead’s script schema (minussrc), plus Unheaddata-*keys / schema augmentations, etc. - where each property is “resolvable” via
ResolvableProperties<T>(i.e., values can be plain values,Ref,ComputedRef, or a getter function). [1][2]
ResolvableProperties is (simplified) mapped as:
type ResolvableProperties<T> = { [key in keyof T]?: ResolvableValue<T[key]> }and ResolvableValue<T> includes T | (() => T) | Ref<T> | ComputedRef<T> (plus falsy allowances). [2]
Example
useScript({
src: 'https://example.com/sdk.js',
async: true,
defer: () => true,
// ...other script attributes supported by Unhead
})[1][2]
Sources: @unhead/vue/dist/scripts.d.mts (UseScriptInput) [1], @unhead/vue/dist/shared/vue.DoxLTFJk.d.mts (ResolvableProperties/ResolvableValue) [2].
🏁 Script executed:
# Let's also check if there's a test that validates the typing of useScript with strings
rg "useScript.*string" test/ --type ts -B 3 -A 3Repository: nuxt/scripts
Length of output: 489
🏁 Script executed:
# Check if there's a TypeScript compilation step that would catch this
cat tsconfig.json 2>/dev/null || cat tsconfig.app.json 2>/dev/null || echo "No tsconfig found in root"Repository: nuxt/scripts
Length of output: 387
Preserve bare-string useScript() calls in the type signature.
The NuxtUseScriptInput type definition narrows away the string branch from UseScriptInput. In TypeScript, the intersection (string | Object) & Record<string & {}, any> distributes to (string & Record<string & {}, any>) | (Object & Record<string & {}, any>), and since string cannot intersect with an object type, the string branch is eliminated. This breaks all existing useScript('https://...') calls at compile time, even though the runtime at line 36 still normalizes string input. The pattern is widely used throughout tests and documentation.
Explicitly preserve the string union in NuxtUseScriptInput:
Suggested fix in src/runtime/types.ts
- export type NuxtUseScriptInput = UseScriptInput & Record<string & {}, any>
+ type ObjectUseScriptInput = Exclude<UseScriptInput, string>
+ export type NuxtUseScriptInput = string | (ObjectUseScriptInput & Record<string & {}, any>)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/composables/useScript.ts` around lines 35 - 36, The type
NuxtUseScriptInput has been defined so it narrows away the string branch, which
breaks existing useScript('...') calls even though useScript normalizes string
at runtime; update the NuxtUseScriptInput type to explicitly preserve the string
union (e.g., NuxtUseScriptInput = string | (UseScriptInput & Record<string,
unknown>)) so the overload/union keeps bare-string calls while still accepting
the object form—adjust the exported NuxtUseScriptInput type and any related
imports/exports to use this union without changing the runtime normalization in
useScript.
Only allow string | boolean | number | undefined for custom attributes, matching what HTML attributes actually accept.
1 participant
🔗 Linked issue
Resolves #292
❓ Type of change
📚 Description
Non-
data-*attributes on script tags (e.g.tw-client-key) caused TypeScript errors because theUseScriptInputtype only alloweddata-*keys. ExtendsNuxtUseScriptInputwithRecord<string & {}, any>to accept arbitrary attributes while preserving autocomplete for known properties.