Skip to content

Conversation

@gayanMatch
Copy link

Summary

  • Move React-style camelCase SVG attributes (e.g., strokeWidth, fillOpacity) from core to compat only
  • Core Preact now only exposes standard kebab-case SVG attributes (e.g., stroke-width, fill-opacity)
  • Add CamelCaseSVGAttributes interface in compat for React compatibility
  • Create CompatJSX namespace that extends core JSX with camelCase SVG support

Test plan

  • Core TypeScript tests verify camelCase SVG props cause type errors
  • Compat TypeScript tests verify camelCase SVG props work correctly
  • npm run test:ts passes

Closes #4994

Contribution by Gittensor, see my contribution statistics at https://gittensor.io/miners/details?githubId=132382032

Copy link
Member

@rschristian rschristian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! Few comments:

@gayanMatch
Copy link
Author

Hi @rschristian ,
I updated PR with your feedback. Please review it.
The changes I made are

  1. Renamed compat/src/svg-camel-case.d.ts to compat/src/dom.d.ts - matching core's pattern, and renamed the interface from CamelCaseSVGAttributes to CompatSVGAttributes with an updated comment
  2. Created compat/src/jsx.d.ts - moved the CompatJSX namespace here with CompatIntrinsicSVGElements inside it, mirroring what core does with src/jsx.d.ts
  3. Updated compat/src/index.d.ts - imports from the new dom.d.ts and jsx.d.ts files, removed the inline namespace definitions
  4. Moved SVG tests from test/ts/preact.tsx to test/ts/dom-attributes.test-d.tsx - core SVG attribute tests (which verify camelCase attributes are NOT available in core)
  5. Reverted package-lock.json - removed accidental changes
  6. Created compat/test/ts/dom-attributes.test-d.tsx - moved compat SVG tests here from compat/test/ts/index.tsx (which verify camelCase attributes ARE available in compat)

@rschristian
Copy link
Member

I gave this a test by installing into a separate project and it doesn't seem to be functioning correctly, the JSX namespace is not updated when preact/compat is used:

// When the following line is uncommented, the type error should disappear
// import 'preact/compat';

const fillOpacityTest = <rect fillOpacity="0.5" />;

The tsconfig.json within compat/test/ts has "jsx": "react" set, which is probably hiding this problem (and, IMO, we shouldn't be doing, but that's a separate issue).

@gayanMatch
Copy link
Author

Hi @rschristian ,
Changeds made:

  1. compat/src/index.d.ts - Added global JSX namespace augmentation so import 'preact/compat' updates the JSX types
  2. compat/jsx-runtime.d.ts - New file that exports CompatJSX as JSX for jsxImportSource: "preact/compat"
  3. compat/jsx-dev-runtime.d.ts - Same for dev runtime

These changes ensure the JSX namespace is properly updated in both scenarios:

  • Using jsxImportSource: "preact/compat"
  • Using import 'preact/compat' with classic JSX transform

@rschristian
Copy link
Member

Thanks! I'll have to test yet, but can you explain the need for the jsx-runtime.d.ts & jsx-dev-runtime.d.ts files? Not quite sure I understand why they'd be needed. Maybe if a third-party React library only used the jsx-runtime, so that's the only thing that'd get aliased perhaps?

@gayanMatch
Copy link
Author

gayanMatch commented Jan 12, 2026

The jsx-runtime.d.ts files would be needed specifically when:

  1. jsxImportSource: "preact/compat" - If someone explicitly sets this in their tsconfig, TypeScript looks for the JSX export from preact/compat/jsx-runtime for type checking (not the global namespace).
  2. Aliasing react/jsx-runtime → preact/compat/jsx-runtime - Your hypothesis is correct. If a bundler aliases the jsx-runtime specifically to compat's version, the types need to come from there.

If you want to test without them first and only add if needed, that's reasonable. The global augmentation might be sufficient if:

  • Most users use jsxImportSource: "preact" (not "preact/compat")
  • The compat import augments the global JSX namespace which TypeScript falls back to.

Do you want me to remove them and just keep the global augmentation?

Copy link
Member

@rschristian rschristian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. jsxImportSource: "preact/compat"

This is not a concern, no one should be doing this and we don't document it anywhere.


Adding a dedicated compat/jsx-runtime.d.ts module makes sense to me as it should cover the situations in which a node module uses only the JSX runtime import from "react" and nothing else. Along those lines though, it's likely that all top-level .d.ts files within compat will need similar re-exporting, so client.d.ts & server.d.ts primariliy.

@gayanMatch
Copy link
Author

gayanMatch commented Jan 13, 2026

  • Removed jsx-dev-runtime.d.ts (using same types file for both runtimes)
  • Updated package.json types entries for compat/jsx-runtime and compat/jsx-dev-runtime to point to ./compat/jsx-runtime.d.ts

@rschristian
Copy link
Member

rschristian commented Jan 15, 2026

Did you try using this?

// When the following line is uncommented, the type error should disappear
// import 'preact/compat';

const fillOpacityTest = <rect fillOpacity="0.5" />;

Perhaps I'm being daft, but doesn't seem to work as expected, even with the latest changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSX types reflect core + compat, when compat may or may not be used

2 participants