[Fix] destructuring-assignment: fix a false prositive for local prop named context in SFC
#2912
Conversation
| get propsName() { | ||
| for (const params of queue) { | ||
| const props = params[0]; | ||
| if (props && !props.destructuring && props.name) { | ||
| return props.name; | ||
| } | ||
| } | ||
| return null; | ||
| }, | ||
| get contextName() { | ||
| for (const params of queue) { | ||
| const context = params[1]; | ||
| if (context && !context.destructuring && context.name) { | ||
| return context.name; | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
getters are slow and the indirection is less maintainable; let's just make these functions.
| get propsName() { | |
| for (const params of queue) { | |
| const props = params[0]; | |
| if (props && !props.destructuring && props.name) { | |
| return props.name; | |
| } | |
| } | |
| return null; | |
| }, | |
| get contextName() { | |
| for (const params of queue) { | |
| const context = params[1]; | |
| if (context && !context.destructuring && context.name) { | |
| return context.name; | |
| } | |
| } | |
| return null; | |
| } | |
| propsName() { | |
| for (const params of queue) { | |
| const props = params[0]; | |
| if (props && !props.destructuring && props.name) { | |
| return props.name; | |
| } | |
| } | |
| return null; | |
| }, | |
| contextName() { | |
| for (const params of queue) { | |
| const context = params[1]; | |
| if (context && !context.destructuring && context.name) { | |
| return context.name; | |
| } | |
| } | |
| return null; | |
| } |
There was a problem hiding this comment.
ok, but why getters are slow and the indirection is less maintainable, please tell me more detail. please do tell me!
There was a problem hiding this comment.
They’re slow because it takes more effort for the engine to use them; they’re less maintainable because it looks like a property at the usage site, but it’s actually invoking a function. Making it a real function means you add () and suddenly everything is faster and more obvious. Getters and setters are bad and should be avoided.
There was a problem hiding this comment.
I haven't contributed code to this issue I originally reported (thanks @SyMind), but I am just reviewing what has been done so far and I feel compelled to say: that's a bad take. Getters and setters are quite a bit more performant than you might think (I use them in real-time applications even with emitters/streams to great effect), but I can respect that since you maintain this project you may have a different opinion and be within your rights to enforce your opinion.
| @@ -38,21 +76,22 @@ module.exports = { | |||
| create: Components.detect((context, components, utils) => { | |||
| const configuration = context.options[0] || DEFAULT_OPTION; | |||
| const ignoreClassFields = (context.options[1] && (context.options[1].ignoreClassFields === true)) || false; | |||
| const sfcParams = createSFCParams(); | |||
There was a problem hiding this comment.
hm, does eslint itself not have utilities for determining the relationship to function params?
…p named `context` in SFC. Fixes #2911.
|
@SyMind are you no longer interested in completing this PR? it's just awaiting some tweaks. |
|
Although it looks like you deleted your fork, which is very sad, because I think that's an unrecoverable state. |
|
@ljharb Sorry, I did a stupid |
|
ok, thanks |
Fixes #2911.