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

Error withforbidExtraProps and scoped.css? #51

Open
Andreas-Sujono opened this issue Feb 25, 2021 · 9 comments
Open

Error withforbidExtraProps and scoped.css? #51

Andreas-Sujono opened this issue Feb 25, 2021 · 9 comments

Comments

@Andreas-Sujono
Copy link

I use scoped.css where everytime I import the css, it will append some random identifier to the component as the css selector. This will create an error to this library because of the forbidExtraProps function

@ljharb
Copy link
Collaborator

ljharb commented Feb 25, 2021

What random identifier? Can you provide some example code?

@Andreas-Sujono
Copy link
Author

something like this:
Screen Shot 2021-02-26 at 1 43 13 AM

but what is the rationale to add the prototypes check on the library? I don't think it's necessary

@ljharb
Copy link
Collaborator

ljharb commented Feb 25, 2021

That looks like a data attribute on an HTML element, not something on the outside click handler component.

It's quite necessary, and every component should have it. The rationale is that it avoids bugs - if you meant to pass foo but accidentally pass fooo, you'll get a propType warning.

@Andreas-Sujono
Copy link
Author

Andreas-Sujono commented Feb 25, 2021

yeah, that's just the example of the scoped.css library that I'm using. So for the outside click handler component, it will be similar with that (<div data-v-b26f6997>).

Because for every other npm libraries that I'm using, none of them does the prototypes checking. Because essentially, the prototype checking only relevant for the shared component that you're building by yourself. Even in the real practice, you need to remove the prototype in the production file. (correct me if I'm wrong)
Because usually, for a library, they have their own documentation about the props and the type. and that's more than enough

https://www.npmjs.com/package/babel-plugin-transform-react-remove-prop-types

@ljharb
Copy link
Collaborator

ljharb commented Feb 25, 2021

You're wrong; React itself doesn't check propTypes in production, but propTypes themselves do always evaluate in production, and it's very tricky to remove them safely.

Documentation is not enough whatsoever.

Either way, the reason this prop is "extra" is because it has no effect except to perhaps allow you to target it with CSS, and this library very intentionally does not allow you that kind of styling control.

@Andreas-Sujono
Copy link
Author

okay fair enough, you're more experience in React.

maybe my last question is why did you use the forbidExtraProps?

because forbidExtraProps is the one that creates error on my side. And across the hundreds of other npm libraries that I'm using, none of them throws error which means none of them using forbidExtraProps.

@ljharb
Copy link
Collaborator

ljharb commented Feb 25, 2021

As far as I know, Airbnb uses that on every single component they have (or the equivalent TypeScript type), and the purpose is to prevent bugs. If you are passing an extra prop, your code is broken.

@Andreas-Sujono
Copy link
Author

Okay, thanks.

just last thought of mine, I cannot find a single use case where you pass an extra props and make the code broken.
and sorry, I'm a newbie in the typescript, but what is the equivalent code for the forbidExtraProps in the typescript?

@ljharb
Copy link
Collaborator

ljharb commented Feb 25, 2021

I think by default, passing an extra prop to a component is a type error, when the component has prop types defined.

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

No branches or pull requests

2 participants