chore: add inline new class warning #9583
Conversation
🦋 Changeset detectedLatest commit: 0416fc8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const performance = { | ||
| 'inline-new-class': () => | ||
| `Creating inline classes will likely cause performance issues. ` + | ||
| `Instead, declare the class at the module-level and create new instances from the class reference.` |
There was a problem hiding this comment.
Should we have something in here like "or use getters/setters instead" because many people use it as a shortcut for this?
There was a problem hiding this comment.
Do we want to tell people to use getters/setters here, or to just hoist their class?
This comment was marked as resolved.
This comment was marked as resolved.
packages/svelte/tests/validator/samples/inline-new-class-2/input.svelte
Outdated
Show resolved
Hide resolved
|
I was going to suggest this should be an eslint warning instead. Inline classes are simpler to write and there are a lot of basic projects out there that don't need the improved performance |
|
@benmccann It's not just performance (however, if you are running without a JIT, i.e. on iOS in Chrome/FF/Facebook/TikTok etc, then the cost is gigantic), but there's quite a big memory impact from creating a new class each time. It's just a bad pattern that we shouldn't promote. If people are using inline classes because they're simpler to write, then we need to address that problem separately. |
|
If the heuristic we're using is 'declaration must be hoisted to component/module' rather than 'don't instantiate in the same scope as the declaration' then the warning should go on the declaration. I'll open a new PR |
Creating classes inline can lead to significant performance memory problems and we should try and help people move away from this problematic pattern in favour of putting the class as a top-level module declaration, or just using object literals instead.