Migrate JavaScript to ES2015 classes #3771
Conversation
colinrotherham
force-pushed
the
module-classes
branch
from
d106d0c
to
be2c55d
Compare
colinrotherham
changed the title
colinrotherham
changed the title
|
Had a few TypeScript checks fail because our tests access Assume we're alright doing what we do elsewhere and ignoring known violations? // @ts-expect-error Parameter 'count' not a number
expect(i18n.getPluralSuffix('test', 'nonsense')).toBe('other')Note: Ideally we should update our tests to access only public methods, perhaps making getters/setters for things like |
colinrotherham
force-pushed
the
module-classes
branch
2 times, most recently
from
69fedde
to
a44fccd
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-br-module-class
|
Had to make a manual Heroku review app: Automatic one is still stuck from Heroku downtime yesterday 
|
| // Code goes here | ||
| } | ||
| ``` | ||
|
|
||
| Assign methods to the prototype object. Do not overwrite the prototype with a new object as this makes inheritance impossible. | ||
| Add methods to the class. Do not overwrite the prototype with a new object as this makes inheritance impossible. |
There was a problem hiding this comment.
I think we can delete this bit as it's no longer relevant?
| Add methods to the class. Do not overwrite the prototype with a new object as this makes inheritance impossible. | |
| Add methods to the class. |
There was a problem hiding this comment.
Left it in as a technicality (still possible I guess?) but also glad to remove!
There was a problem hiding this comment.
Removed it ✅
|
|
||
| /** @deprecated Will be made private in v5.0 */ | ||
| this.$module = $module | ||
| export class Details { |
There was a problem hiding this comment.
Worth noting that this file is being deleted in #3766 – could remove the changes relating to this file to avoid potential unnecessary conflicts, unless that makes ESLint unhappy?
There was a problem hiding this comment.
I'm happy to fix conflicts 🤷♂️
There was a problem hiding this comment.
Let's try and merge the other PR first
There was a problem hiding this comment.
As fallback styling for Details in older browsers is still in progress let's switch and merge this first
We prefer named exports over default exports to avoid compatibility issues with transpiler "synthetic default" as discussed in: #2829 See our [coding standards for JavaScript](/docs/contributing/coding-standards/js.md)
This hides them from the generated JSDoc output too
Ideally we should update our tests to access only public methods, perhaps making getters/setters for things like `getCountMessage()` instead?
This is a “just in case” commit where early ES2015 class implementations would return `undefined` in a subclass constructor (extended component) rather than the subclass instance Might only affect Safari 9 See: #2987 (comment)
colinrotherham
force-pushed
the
module-classes
branch
from
a44fccd
to
f16f23d
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-br-module-class
Now we've added
<script type="module">we can use ES2015 (or ES6) classes for components in #3506Make sure you view Files changed with "Hide whitespace" ticked as class methods are mostly only indented