fix: type CriticalScript props as script attributes

[gwagjiug]

Purpose

Modules imported with the ?as-critical-script suffix are exposed as React components that render a <script> element.

The documentation states that these components accept the standard HTML <script> attributes. However, the public type declaration previously extended React’s generic HTMLAttributes:

interface CriticalScriptProps extends React.HTMLAttributes<HTMLScriptElement> {}

This allowed attributes shared by most HTML elements, such as id, className, style, and nonce, but omitted attributes specific to <script> elements, including:

• async
• defer
• src
• type
• integrity
• crossOrigin
• noModule
• referrerPolicy
• fetchPriority

As a result, valid component usage produced TypeScript errors:

<CriticalScript
  async
  defer
  integrity="sha384-example"
  src="/critical.js"
  type="module"
/>

For example, TypeScript reported:

Property 'async' does not exist on type
'HTMLAttributes<HTMLScriptElement>'

This was a type declaration mismatch rather than a missing runtime capability. The generated component already forwards all supplied props to the rendered <script> element:

return _jsx('script', { ...props, ...otherProps })

JavaScript consumers, or TypeScript consumers bypassing the incorrect declaration, could therefore already pass these attributes at runtime.

Root cause

React.HTMLAttributes<T> represents attributes shared by general HTML elements. Passing HTMLScriptElement as its generic parameter does not add script-specific attributes; the parameter mainly describes the target element used by event-related types.

React provides a separate interface for <script> elements:

interface ScriptHTMLAttributes<T> extends HTMLAttributes<T> {
  async?: boolean
  defer?: boolean
  integrity?: string
  src?: string
  type?: string
  // ...
}

The previous declaration therefore selected the generic element interface even though the component always renders a <script>.

The existing documentation examples used id and nonce, both of which are already included in the generic HTMLAttributes interface. Those examples passed type checking and did not expose the missing script-specific attributes.

Main changes

Use React’s script-specific attribute interface

CriticalScriptProps now extends React.ScriptHTMLAttributes<HTMLScriptElement>:

interface CriticalScriptProps extends React.ScriptHTMLAttributes<HTMLScriptElement> {}

ScriptHTMLAttributes already extends HTMLAttributes, so this is a type widening rather than a replacement of existing capabilities.

Existing props continue to work:

<CriticalScript id="my-script" nonce={nonce} />

Script-specific props are now also recognized:

<CriticalScript async defer type="module" />

This uses React’s canonical type for a <script> element instead of maintaining a separate attribute list.

Add a compile-time regression check

A small type-check fixture imports an actual ?as-critical-script module and verifies its inferred component props:

import type CriticalScript from './__fixtures__/sample.ts?as-critical-script'

void ({ async: true } satisfies React.ComponentProps<typeof CriticalScript>)

async was selected because it clearly distinguishes ScriptHTMLAttributes from the generic HTMLAttributes interface.

The check demonstrates the regression directly:

Previous declaration:
TS2353: 'async' does not exist in type
'HTMLAttributes<HTMLScriptElement>'

Updated declaration:
No type error

The fixture is included in the package’s existing tsc --noEmit check, so no new test framework, script, or configuration is required. It uses a type-only import and is not part of the package’s runtime build output.

Why this is not treated as an intentional restriction

It is possible in principle that a critical inline-script component might intentionally restrict attributes such as src, async, or defer.

However, the repository currently provides no evidence of such a policy:

• The declaration and the original documentation were introduced together.
• The documentation has stated from the first commit that the component accepts standard HTML <script> attributes.
• The runtime implementation forwards otherProps without filtering script-specific attributes.
• There is no Omit type, allowlist, runtime guard, warning, comment, or test describing prohibited attributes.
• The previous type still allowed general props such as children and dangerouslySetInnerHTML, so it did not form a consistent runtime safety boundary.

Some script attributes may not be useful for every critical inline script. For example, src can change how the browser handles inline content, while async and defer have context-dependent behavior.

This PR does not introduce a new policy for those attributes. It aligns the public type declaration with the existing documentation and runtime behavior.

If the project later decides to prohibit specific attributes, that should be handled as an explicit design change with:

• a documented list of unsupported attributes;
• a targeted TypeScript type such as an Omit;
• corresponding runtime filtering or validation;
• tests covering the intended restriction.

User-visible behavior

Behavior	Before	After
Common attributes such as id and nonce	Accepted	Accepted
Script-specific attributes such as async and type	TypeScript error	Accepted
IDE autocomplete for script attributes	Missing	Available
Runtime prop forwarding	Supported	Unchanged
Generated JavaScript	Unchanged	Unchanged

The change only corrects the TypeScript representation of behavior that the component already supports.

Compatibility and scope

This PR:

• preserves all props previously inherited from HTMLAttributes;
• adds the script-specific props provided by React;
• does not change component rendering;
• does not change prop precedence or runtime filtering;
• does not change the generated inline script;
• does not add runtime dependencies;
• does not change any plugin options;
• does not update documentation because the existing documentation already describes the intended behavior.

This is a source-compatible type widening. Existing consumers do not need to change their code.

Verification

• pnpm --filter @woowabros/vite-plugin-critical-script typecheck
• Confirmed the regression fixture fails with TS2353 when the previous HTMLAttributes declaration is restored
• Confirmed the same fixture passes with ScriptHTMLAttributes
• pnpm --filter @woowabros/vite-plugin-critical-script test — 8 tests passed
• pnpm --filter @woowabros/vite-plugin-critical-script build
• ESLint passed for script-props.typecheck.ts
• Prettier passed for all changed files
• git diff --check upstream/main...HEAD

Summary by CodeRabbit

• Bug Fixes
  • Improved type support for script-specific properties when using the critical script component.
  • Added validation to ensure the async attribute is accepted correctly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: aa183bed-8ead-409e-b00f-3b23147cd8d2

📥 Commits

Reviewing files that changed from the base of the PR and between 1726aed and be226bb.

📒 Files selected for processing (2)

• packages/vite-plugin-critical-script/global.d.ts
• packages/vite-plugin-critical-script/script-props.typecheck.ts

---

📝 Walkthrough

Walkthrough

The PR updates CriticalScriptProps to use React’s script-specific HTML attributes and adds a compile-time assertion that the async prop is accepted.

Changes

Critical script props

Layer / File(s)	Summary
Script prop contract and validation packages/vite-plugin-critical-script/global.d.ts, packages/vite-plugin-critical-script/script-props.typecheck.ts	CriticalScriptProps now extends React.ScriptHTMLAttributes<HTMLScriptElement>. A typecheck assertion verifies that CriticalScript accepts async: true.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to be226

This change widens CriticalScript props to correctly support standard script attributes without changing runtime rendering or generated JavaScript. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: jaehuiui, solo5star

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the change to type CriticalScript props as script attributes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[spoyodevelop]

Just a thought — since otherProps is spread onto the <script> tag, passing things like src, children, or dangerouslySetInnerHTML could unintentionally break or override the inlined script.

Might it be worth omitting them from the props type?

interface CriticalScriptProps
  extends Omit<React.ScriptHTMLAttributes<HTMLScriptElement>, 'src' | 'children' | 'dangerouslySetInnerHTML'> {}

[gwagjiug]

Just a thought — since otherProps is spread onto the <script> tag, passing things like src, children, or dangerouslySetInnerHTML could unintentionally break or override the inlined script.

Might it be worth omitting them from the props type?

interface CriticalScriptProps
  extends Omit<React.ScriptHTMLAttributes<HTMLScriptElement>, 'src' | 'children' | 'dangerouslySetInnerHTML'> {}

@spoyodevelop
Thanks for raising this. I had been concerned about this part as well. A type-level restriction cannot prevent every case—plain JavaScript or as any can still bypass it—but I think omitting src, children, and dangerouslySetInnerHTML from the public type would help prevent accidental misuse. Would it make sense to update the props type as you suggested?

[spoyodevelop]

@gwagjiug
I thought the original maintainer might have intentionally chosen HTMLAttributes to avoid exposing script props that don't fit an inline critical script.

However, since useful props like fetchPriority only exist in ScriptHTMLAttributes, I agree this PR is valid too.

But still, I think we should definitely omit the props that can actually break the inlined script (src, children, dangerouslySetInnerHTML). yup. It may good to update the prop types like i suggested, can make this PR more valid In my opinion.

also, sorry for quiet editing in my comment, I am typing this english with my hands and seems LLM brakes my original intention of my suggestion!

[solo5star]

It is reasonable for the exported component to expose props appropriate for a <script> element. However, extending the full ScriptHTMLAttributes interface would expose attributes that are either invalid for inline scripts or incompatible with critical-script’s intended use.

For the inline classic scripts currently generated by this plugin:

• src would turn the element into an external script and cause the inline content to be ignored.
• defer, integrity, and fetchPriority do not apply without an external script resource.
• async does not apply to inline classic scripts, although it can affect inline module scripts.

We could consider supporting type="module", since inline ESM is valid. However, module scripts are deferred by default, which works against this plugin’s goal of running critical code as early as possible. Loading additional modules from a critical script would also fall outside the intended responsibility of this plugin and should generally be handled by the main JavaScript bundle.

crossOrigin and referrerPolicy are valid on inline scripts, but they only affect module imports, including dynamic import(). Since loading additional modules from a critical script is not an intended use case, I don’t think these attributes are a good fit for the public API either.

noModule may be useful when providing different scripts for environments with and without ESM support.

[solo5star]

I also agree with @spoyodevelop’s suggestion. Callers should not be able to override props that define or replace the inline script content, as doing so could break the component’s intended behavior.

[spoyodevelop]

@solo5star

It is reasonable for the exported component to expose props appropriate for a <script> element. However, extending the full ScriptHTMLAttributes interface would expose attributes that are either invalid for inline scripts or incompatible with critical-script’s intended use.

Ah, I was guessing that way a bit too. I also thought that using HTMLAttributes conveniently works out here because it naturally leaves out those incompatible props.

We could consider supporting type="module", since inline ESM is valid. However, module scripts are deferred by default, which works against this plugin’s goal of running critical code as early as possible. Loading additional modules from a critical script would also fall outside the intended responsibility of this plugin and should generally be handled by the main JavaScript bundle.

I was originally wondering if supporting type="module" could be a reason to use the full ScriptHTMLAttributes. But your point makes a lot of sense. Given this library's context (like ideally keeping the module around 8kb max), simply adding type="module" could break things due to deferred execution and add confusion around using multiple modules.

Thanks for looking into this, it really cleared up my assumptions. Based on what you found, I think sticking with the original code is probably more good for this use case.

[gwagjiug]

@solo5star

Thanks for the detailed explanation. That distinction makes sense to me. I agree that exposing the full ScriptHTMLAttributes interface would make the public API imply support for external-script and module-related behavior that does not fit the inline classic scripts generated by this plugin.

Taking both of your comments together, would the following narrower type better match the intended public API?

interface CriticalScriptProps extends React.HTMLAttributes<HTMLScriptElement> {
  children?: never
  dangerouslySetInnerHTML?: never
}

This would keep script-specific attributes such as src, async, defer, and type outside the public API, while preventing their accidental use through the public TypeScript API when they could conflict with or replace the generated inline content.

I used explicit never properties rather than Omit alone because the package supports @types/react 17, where React.FC adds children implicitly. This keeps the restriction consistent across the supported React type versions.

If this matches the direction you have in mind, I’ll narrow the PR accordingly and update its type-level checks, description, and related documentation wording.