Skip to content

Commit f8343ad

Browse files
authored
Merge pull request #46 from github/docs-add-interactive-name-checker
docs: add convention/anti-pattern around shadowing HTMLElement names
2 parents 8d70063 + 514b4ed commit f8343ad

3 files changed

Lines changed: 95 additions & 28 deletions

File tree

docs/_guide/anti-patterns.md

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ chapter: 9
33
subtitle: Anti Patterns
44
---
55

6-
{% capture discouraged %}<h4 class="text-red"><svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24"><path fill-rule="evenodd" d="M1 12C1 5.925 5.925 1 12 1s11 4.925 11 11-4.925 11-11 11S1 18.075 1 12zm8.036-4.024a.75.75 0 00-1.06 1.06L10.939 12l-2.963 2.963a.75.75 0 101.06 1.06L12 13.06l2.963 2.964a.75.75 0 001.061-1.06L13.061 12l2.963-2.964a.75.75 0 10-1.06-1.06L12 10.939 9.036 7.976z"></path></svg> Discouraged</h4>{% endcapture %}
7-
8-
{% capture encouraged %}<h4 class="text-green"><svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24"><path fill-rule="evenodd" d="M1 12C1 5.925 5.925 1 12 1s11 4.925 11 11-4.925 11-11 11S1 18.075 1 12zm16.28-2.72a.75.75 0 00-1.06-1.06l-5.97 5.97-2.47-2.47a.75.75 0 00-1.06 1.06l3 3a.75.75 0 001.06 0l6.5-6.5z"></path></svg> Encouraged</h4>{% endcapture %}
6+
{% capture octx %}<svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24"><path fill-rule="evenodd" d="M1 12C1 5.925 5.925 1 12 1s11 4.925 11 11-4.925 11-11 11S1 18.075 1 12zm8.036-4.024a.75.75 0 00-1.06 1.06L10.939 12l-2.963 2.963a.75.75 0 101.06 1.06L12 13.06l2.963 2.964a.75.75 0 001.061-1.06L13.061 12l2.963-2.964a.75.75 0 10-1.06-1.06L12 10.939 9.036 7.976z"></path></svg>{% endcapture %}
7+
{% capture octick %}<svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24"><path fill-rule="evenodd" d="M1 12C1 5.925 5.925 1 12 1s11 4.925 11 11-4.925 11-11 11S1 18.075 1 12zm16.28-2.72a.75.75 0 00-1.06-1.06l-5.97 5.97-2.47-2.47a.75.75 0 00-1.06 1.06l3 3a.75.75 0 001.06 0l6.5-6.5z"></path></svg>{% endcapture %}
8+
{% capture discouraged %}<h4 class="text-red">{{ octx }} Discouraged</h4>{% endcapture %}
9+
{% capture encouraged %}<h4 class="text-green">{{ octick }} Encouraged</h4>{% endcapture %}
910

1011
Here are a few common anti-patterns which we've discovered as developers have used Catalyst. We consider these anti-patterns as they're best avoided, because of surprising edge-cases, or simply because there are easier ways to achieve the same goals.
1112

@@ -111,6 +112,59 @@ class UserSettingsElement extends HTMLElement {
111112
</div>
112113
</div>
113114

115+
### Avoid shadowing method names
116+
117+
When naming a method, you should avoid naming it something that already exists on the `HTMLElement` prototype; as doing so can lead to surprising behaviors. Test out the form below to see what method names are allowed or not:
118+
119+
<form>
120+
<label>
121+
<h4>I want my method to be called...</h4>
122+
<input class="js-methodname-shadow-test mb-4">
123+
</label>
124+
<div hidden class="js-methodname-shadow-bad-input text-red">
125+
{{ octx }} This name would shadow <code></code>, you'll need to pick a different name
126+
</div>
127+
<div hidden class="js-methodname-shadow-warn-input text-orange-light">
128+
{{ octx }} While this name is allowed, it's not ideal because <span></span>. You should consider a different name.
129+
</div>
130+
<div hidden class="js-methodname-shadow-good-input text-green">
131+
{{ octick }} This is a good name for a method!
132+
</div>
133+
<script>
134+
const warnings = {
135+
'new': 'it has a special meaning in JS',
136+
'super': 'it has a special meaning in JS',
137+
'prototype': 'it has a special meaning in JS',
138+
'requestSubmit': 'it is a proposed new feature',
139+
}
140+
document.querySelector('.js-methodname-shadow-test').addEventListener('input', () => {
141+
const name = event.target.value
142+
const goodEl = document.querySelector('.js-methodname-shadow-good-input')
143+
const badEl = document.querySelector('.js-methodname-shadow-bad-input')
144+
const warnEl = document.querySelector('.js-methodname-shadow-warn-input')
145+
let warning = warnings[name]
146+
if (name !== name.toLowerCase() && name.toLowerCase() in HTMLElement.prototype) {
147+
warning = `it is too similar to \`${name.toLowerCase()}\` which already exists`
148+
} else if (name.startsWith('on') && !(name in HTMLElement.prototype)) {
149+
warning = 'starting with `on` suggests a coupling between the event and the method (see below)'
150+
}
151+
goodEl.hidden = warning || (name in HTMLElement.prototype)
152+
warnEl.hidden = !warning
153+
badEl.hidden = warning || !(name in HTMLElement.prototype)
154+
if (warning) {
155+
warnEl.querySelector('span').textContent = warning
156+
} else if (name in HTMLElement.prototype) {
157+
let proto = HTMLElement.prototype
158+
while(proto !== null) {
159+
if (proto.hasOwnProperty(name)) break
160+
proto = Object.getPrototypeOf(proto)
161+
}
162+
badEl.querySelector('code').textContent = `${proto.constructor.name}.prototype.${name}`
163+
}
164+
})
165+
</script>
166+
</form>
167+
114168
### Avoid naming methods after events, e.g. `onClick`
115169

116170
When you have a method which is only called as an event, it is tempting to name that method based off of the event, e.g. `onClick`, `onInputFocus`, and so on. This name implies a coupling between the event and method, which later refactorings may break. Also names like `onClick` are very close to `onclick` which is already [part of the Element's API](https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onclick). Instead we recommend naming the method after what it does, not how it is called, for example `resetForm`:

docs/_guide/conventions.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class UserListElement extends HTMLElement {}
1616

1717
### The best class-names are two word descriptions
1818

19-
Custom elements are required to have a `-` inside the tag name. Catalyst's `@controller` will derive the tag name from the class name - and so as such the class name needs to have at least two capital letters, or to put it another way, it needs to consist of two words. The element name should describe what it does succinctly in two words. Some examples:
19+
Custom elements are required to have a `-` inside the tag name. Catalyst's `@controller` will derive the tag name from the class name - and so as such the class name needs to have at least two capital letters, or to put it another way, it needs to consist of at least two CamelCased words. The element name should describe what it does succinctly in two words. Some examples:
2020

2121
- `theme-picker` (`class ThemePickerElement`)
2222
- `markdown-toolbar` (`class MarkdownToolbarElement`)
@@ -34,8 +34,8 @@ Be careful not to go too short! We'd recommend avoiding contracting words such a
3434

3535
### Method names should describe what they do
3636

37-
A good method name, much like a good class name, describes what it does, not how it was invoked. While methods can be given any name, names like `onClick` are best avoided, overly generic names like `toggle` should also be avoided. Just like class names it is a good idea to ask "how" and "what", so for example `showAdmins`, `filterUsers`, `updateURL`.
37+
A good method name, much like a good class name, describes what it does, not how it was invoked. While methods can be given most names, you should avoid names that conflict with existing methods on the `HTMLElement` prototype (more on that in [anti-patterns](/guide/anti-patterns#avoid-shadowing-method-names)). Names like `onClick` are best avoided, overly generic names like `toggle` should also be avoided. Just like class names it is a good idea to ask "how" and "what", so for example `showAdmins`, `filterUsers`, `updateURL`.
3838

3939
### `@target` should use singular naming, while `@targets` should use plural
4040

41-
To help differentiate the two `@target`/`@targets` decorators, the properties should be named with respective to their cardinality. That is to say, if you're using an `@target` decorator, then the name should be singular (e.g. `user`, `field`) while the `@targets` decorator should be coupled with plural property names (e.g. `users`, `fields`).
41+
To help differentiate the two `@target`/`@targets` decorators, the properties should be named with respective to their cardinality. That is to say, if you're using an `@target` decorator, then the name should be singular (e.g. `user`, `field`) while the `@targets` decorator should be coupled with plural property names (e.g. `users`, `fields`).

docs/_guide/your-first-component.md

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,51 +3,64 @@ subtitle: Building an HTMLElement
33
chapter: 2
44
---
55

6-
Custom Elements allow you to create reusable components that you can declare in HTML, and [progressively enhance](https://en.wikipedia.org/wiki/Progressive_enhancement) within JavaScript. Custom Elements must named with a `-` in the HTML name, and the JS class must `extend HTMLElement`. When the browser connects each element class instance to the DOM node, `connectedCallback` is fired - this is where you can change parts of the element. Here's a basic example:
6+
### Catalyst's `@controller` decorator
77

8-
```html
9-
<hello-world></hello-world>
10-
<script>
8+
Catalyst's `@controller` decorator lets you create Custom Elements with virtually no boilerplate, by automatically calling `customElements.register`, and by adding ["Actions"](/guide/actions) and ["Targets"](/guide/targets) features described later. Using TypeScript (with `decorators` support enabled), simply add `@controller` to the top of your class:
9+
10+
```js
11+
@controller
1112
class HelloWorldElement extends HTMLElement {
1213
connectedCallback() {
1314
this.innerHTML = 'Hello World!'
1415
}
1516
}
16-
window.customElements.register('hello-world', HelloWorldElement)
17-
</script>
17+
// This happens automatically within `@controller`
18+
// window.customElements.register('hello-world', HelloWorldElement)
1819
```
1920
<br>
2021

22+
Catalyst will automatically convert the classes name; removing the trailing `Element` suffix and lowercasing all capital letters, separating them with a dash.
2123

22-
Here are the three key elements to remember:
24+
By convention, Catalyst controllers end in `Element`, and Catalyst will strip this for the tag name, but the `Element` suffix is not required - just convention. All examples in this guide use `Element` suffixed names.
2325

24-
- Custom Elements must have a dash in the name.
25-
- The JS class must `extend HTMLElement`
26-
- `connectedCallback` can be used as a life-cycle hook for when the element and class are connected.
26+
<div class="d-flex border rounded-1 my-3 box-shadow-medium">
27+
<span class="d-flex bg-blue text-white rounded-left-1 p-3">
28+
<svg width="24" viewBox="0 0 14 16" class="octicon octicon-info" aria-hidden="true">
29+
<path
30+
fill-rule="evenodd"
31+
d="M6.3 5.69a.942.942 0 0 1-.28-.7c0-.28.09-.52.28-.7.19-.18.42-.28.7-.28.28 0 .52.09.7.28.18.19.28.42.28.7 0 .28-.09.52-.28.7a1 1 0 0 1-.7.3c-.28 0-.52-.11-.7-.3zM8 7.99c-.02-.25-.11-.48-.31-.69-.2-.19-.42-.3-.69-.31H6c-.27.02-.48.13-.69.31-.2.2-.3.44-.31.69h1v3c.02.27.11.5.31.69.2.2.42.31.69.31h1c.27 0 .48-.11.69-.31.2-.19.3-.42.31-.69H8V7.98v.01zM7 2.3c-3.14 0-5.7 2.54-5.7 5.68 0 3.14 2.56 5.7 5.7 5.7s5.7-2.55 5.7-5.7c0-3.15-2.56-5.69-5.7-5.69v.01zM7 .98c3.86 0 7 3.14 7 7s-3.14 7-7 7-7-3.12-7-7 3.14-7 7-7z"
32+
/>
33+
</svg>
34+
</span>
35+
<div class="p-3">
36+
37+
Remember! A class name _must_ include at least two CamelCased words (not including the `Element` suffix). One-word elements will raise exceptions. Example of good names: `UserListElement`, `SubTaskElement`, `PagerContainerElement`
2738

28-
### Catalyst
39+
</div>
40+
</div>
2941

30-
Catalyst saves you writing some of this boilerplate, by automatically calling the `customElements.register` code, and by adding ["Actions"](/guide/actions) and ["Targets"](/guide/targets) features described later. If you're using TypeScript with `decorators` support, simply add `@controller` to the top of your class:
3142

32-
```js
33-
@controller
43+
### What does `@controller` do?
44+
45+
Catalyst components are really just "Custom Elements", they're doing all the heavy lifting. Custom Elements allow you to create reusable components that you can declare in HTML, and [progressively enhance](https://en.wikipedia.org/wiki/Progressive_enhancement) within JavaScript. Custom Elements must named with a `-` in the HTML name, and the JS class must `extend HTMLElement`. When the browser connects each element class instance to the DOM node, `connectedCallback` is fired - this is where you can change parts of the element. Here's a basic example:
46+
47+
```html
48+
<hello-world></hello-world>
49+
<script>
3450
class HelloWorldElement extends HTMLElement {
3551
connectedCallback() {
3652
this.innerHTML = 'Hello World!'
3753
}
3854
}
39-
// No longer need this:
40-
// window.customElements.register('hello-world', HelloWorldElement)
55+
window.customElements.register('hello-world', HelloWorldElement)
56+
</script>
4157
```
42-
<br>
43-
44-
Catalyst will automatically convert the classes name; removing the trailing `Element` suffix and lowercasing all capital letters, separating them with a dash.
4558

46-
By convention, Catalyst controllers end in `Element`, and Catalyst will strip this for the tag name, but suffixing `Element` is not required. All examples in this guide use `Element` suffixed names.
59+
The Catalyst version isn't all that different, it's just that we have the `@controller` decorator to save on some of the boilerplate.
4760

48-
#### What about without Decorators?
61+
### What about without TypeScript Decorators?
4962

50-
If you don't want to use decorators, you can simply wrap the class in a call to `controller`:
63+
If you don't want to use TypeScript decorators, you can use `controller` as a regular function, and just pass it your class:
5164

5265
```js
5366
controller(

0 commit comments

Comments
 (0)