-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: warn on obvious legacy component instantiation #12648
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
Conversation
Adds a compiler warning that warns about legacy component instantiation (i.e. using `new Component(..)`). This won't catch all cases, but the most obvious ones which probably make up ~80%
|
False negatives are okay but we should be careful to avoid false positives. Are we concerned that people might rely on extensionless imports, i.e. importing |
In my experience it really depends on the editor and tsconfig setup...I let my editor auto complete and sometimes it import extension less. We could change the generated tsconfig in sveltekit so that import the |
Yeah, I mean it even works in our own playground. I'm not sure we can realistically make this change. This feels like something that should be solved with typechecking, rather than in the compiler itself. |
I forgot the "is a default import" check, which silences all the false positives except the |
Fair enough. Should we go one step further and check that if the sole argument is an object literal, it has a |
yeah sounds good - it's very rare that you would pass a variable into the constructor that takes care of that for you |
Adds a compiler warning that warns about legacy component instantiation (i.e. using
new Component(..)
). This won't catch all cases, but the most obvious ones which probably make up ~80%Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint