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

update docs and message on template arrow and bind #126

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lucaelin
Copy link

@lucaelin lucaelin commented May 3, 2022

Fixes #125 .
I've tried to improve that phrasing on messages and docs that I found confusing, but I am not a native speaker! Feel free to comment on inaccuracies!

@43081j
Copy link
Owner

43081j commented Nov 25, 2022

sorry this got lost and went a bit stale.

i've just read through it again and im wondering if we should actually word it differently.

basically:

no-template-arrow isn't specific to events, its just pointing out that you should store the function somewhere if you can (reference it) rather than creating one each time. maybe we simply need to reword it so it isn't specific to events? since the rule really isn't

i think the same applies to no-template-bind.

both of these rules exist because if you can reference a method/function rather than passing one inline, its usually the better choice. for events, properties, etc.

@lucaelin
Copy link
Author

The behavior described with the current rule description is only true for event listeners. Afaict lit only auto-binds event listeners and not functions passed as properties. This difference in lits binding behavior has caused me (and others) quite a bit of confusion and has ultimately led me to always wrap function calls in an inline arrow to get consistent behavior.
I think it would greatly benefit the community to point out this difference more clearly and let people decide how they ultimately want to mitigate it.

@lucaelin
Copy link
Author

its usually the better choice

from a performance standpoint, yes. From a cleanliness standpoint and for consistency in the codebase this may be a whole different story I'm afraid...

@43081j
Copy link
Owner

43081j commented Nov 27, 2022

The behavior described with the current rule description is only true for event listeners

i think what im saying is the current docs are already unclear.

these two rules are not to do with event listeners. they're just about passing references to functions rather than inlining them. this can be anything - properties, events, etc.

if the current docs say its to do with auto-binding etc, thats the fault i think. both rules should be generic, purely about passing refs rather than inline functions, in any situation.

as for cleanliness etc, its an opinionated rule, thats ok. we just have to make it clear in the docs that if you prefer inline arrow functions, don't use it.

@lucaelin
Copy link
Author

Oh I see now and yes, it is true that you should always try to pass the same value and not create new ones.
I still think there is value in proposing alternatives to in-lining. I assume most people run into these rules issues, because they want to ensure a consistent this-context in their functions, and the solution of the binding problem depends on whether we are talking about function properties, or event-listeners.
Not mentioning this at all is certainly an option, but I'd prefer to be learn something new when using a linter, instead of only being shown the road-block. This is a personal thing, though, and it's up to you ofc.

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

Successfully merging this pull request may close these issues.

lit/no-template-bind is not correct about auto-binding?
2 participants