Skip to content

docs: add convention/anti-pattern around shadowing HTMLElement names#46

Merged
keithamus merged 4 commits intomasterfrom
docs-add-interactive-name-checker
Jul 14, 2020
Merged

docs: add convention/anti-pattern around shadowing HTMLElement names#46
keithamus merged 4 commits intomasterfrom
docs-add-interactive-name-checker

Conversation

@keithamus
Copy link
Copy Markdown
Contributor

@keithamus keithamus commented Jun 24, 2020

This adds some more docs around method naming, per @muan's comment in #45 (comment): it discusses how people should avoid method names that shadow existing HTMLElement properties, also adding an interactive form so people can type out their method name to see it (originally I had a list but it was a bit boring and very long).

Peek 2020-06-24 15-32

It also has warnings for method names which, while allowed, we'd like to not see:

Peek 2020-06-24 17-16

The form is live, and makes a dynamic check against HTMLElement.prototype, it also checks ancestors to see which particular class has the shadowed property, for quicker googling if the user wants to google.

@keithamus keithamus requested a review from a team as a code owner June 24, 2020 14:38
@github-pages github-pages Bot temporarily deployed to github-pages June 24, 2020 14:40 Inactive
Comment thread docs/_guide/anti-patterns.md Outdated
Based on user feedback this section has been re-written for a few
reasons:

 - It focusses more on the `@controller` decorator and less on Custom
   Elements
 - It repeats (multiple times, including a call-out box) that the class
   name must consist of two words, as this is something users miss.
@github-pages github-pages Bot temporarily deployed to github-pages June 24, 2020 16:16 Inactive
Copy link
Copy Markdown
Contributor

@koddsson koddsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I wonder if we could also take the interactive form functionality and package it into a eslint rule.

@github-pages github-pages Bot temporarily deployed to github-pages July 14, 2020 11:25 Inactive
@keithamus keithamus merged commit f8343ad into master Jul 14, 2020
@keithamus keithamus deleted the docs-add-interactive-name-checker branch July 14, 2020 11:28
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.

3 participants