Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No type error when setting an unknown property of a JSX element #42403

Open
trusktr opened this issue Jan 19, 2021 · 7 comments
Open

No type error when setting an unknown property of a JSX element #42403

trusktr opened this issue Jan 19, 2021 · 7 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@trusktr
Copy link
Contributor

trusktr commented Jan 19, 2021

Bug Report

I've augmented solid-js's JSX.IntrinsicElements with new intrinsic elements made within my solid-js-consuming application.

When I write an unknown prop manually (not with spread as detailed in #18801), I do not get a type error.

should-be-error.mp4

🔎 Search Terms

typescript no error on unknown prop

🕗 Version & Regression Information

Not sure when this happened, but I've been trying unsuccessfully to import {JSX} into particular files to as not to clash with React's global JSX definitions, but because import {JSX} does not work (#41813), I've since started trying to use the new @jsxImportSource feature as comments inside my files.

So far, I have gotten types to appear using per-file @jsxImportSource comments, and there are no other global JSX types in those files (I have segregated React JSX code from Solid.js JSX code using Project References and placing the two forms of files in separate projects with their own tsconfig to ensure only certain types are available to each group of files).

So, I'm not sure, but it could be that I'm one of the few people testing unknown waters in TypeScript, and hitting this issue.

I have not tried with an older version of TypeScript, because @jsxImportSource doesn't work there, and import {JSX} has never worked for JSX markup (#41813).

⏯ Playground Link

I tried to recreate the issue in playground, but due to microsoft/TypeScript-Website#1427, @jsxImportSource comments to not work in the playground, so I can't reproduce a small version of my environment.

The following is my attempt to try @jsxImportSource just to get started, but no luck (same example from microsoft/TypeScript-Website#1427):

playground

I'll see if I can make a reproduction in a repo...

💻 Code

As you see in the video, I can hover on an unknown prop, and TypeScript will infer the type of the prop as whatever value I have set it to.

For example, in the next video I changed the value to a number, and now the type shows up as a number instead of a function:

unknown-prop.mp4

I have double checked that the type supplied to JSX.IntrinsicElements does not contain an index signature. There are only specific properties defined in the type of the video's timeline-scrubber.

🙁 Actual behavior

It infers unknown props.

🙂 Expected behavior

It should give a type error.

@MartinJohns
Copy link
Contributor

You forgot to provide example code that allows to reproduce this issue.

@DanielRosenwasser
Copy link
Member

I believe that attributes with - in them are exempt from type-checking. @RyanCavanaugh knows more, but this was probably put in place to accommodate data- attributes.

@RyanCavanaugh
Copy link
Member

The rule here is that properties are allowed to be excess if their names wouldn't be legal variable names. This was the most-agnostic thing we could think of that didn't just allow anything, yet still allowed the entire family of data- and aria- properties to be provided in a certain framework whose name I dare not speak in current company.

Once we have some way of representing those keys in the type system, we'll remove that rule, so duping to #26797

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jan 20, 2021
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@trusktr
Copy link
Contributor Author

trusktr commented Jul 7, 2022

@RyanCavanaugh How does this issue relate to #26797 (which is closed in favor of the merged #44512)? Is it that the new index signature behavior allows to finally fix this issue?

If so, then doesn't this issue need to be open, so that it can be fixed?

I still see the same unwanted behavior happening.

Users (library authors, custom element authors, etc) should be adding all the attribute types they need (to JSX types), and otherwise TS should error on unknown ones as a default.

Someone can easily add an index signature for data-* or aria-* attributes to catch all of them.

Can you please re-open this? 🙏

@RyanCavanaugh RyanCavanaugh reopened this Jul 7, 2022
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Duplicate An existing issue was already created labels Jul 7, 2022
@RyanCavanaugh
Copy link
Member

I think next steps would be:

  • Add the data and aria indexers to the React .d.ts file and other relevant JSX typings
  • Wait a while so that "everyone" is on the latest version of those files
  • Remove the - check

@trusktr
Copy link
Contributor Author

trusktr commented Jan 2, 2024

That will be great. At the same time, if that breaks people, looks like the fix is easy with template literal types:

type O = Record<`data-${string}`, number>

const o: O = {
  'data-foo': 123, // ok
  foo: 123, // error (good)
}

playground

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants