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

Upgrade ESLint configuration for ES2015 (without classes) #3770

Merged
merged 9 commits into from
Jun 8, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Jun 8, 2023

This PR updates our ESLint config as part of #3463 to:

  1. Use ES2015 (or ES6) as our target
  2. Use arrow functions to preserve this in components
  3. Use template strings rather than A + 'to' + B concatenation
  4. Use let or const with block scoping rather than var

I've left ESLint rules in place for .prototype inheritance as we'll move to ES2015 classes in another PR

Browser support

We may go higher than ES2015 once we confirm the browser support guarded by our “cut the mustard” checks

We know that browsers supporting <script type="module"> already support the new features above

supports es6-module
not supports arrow-functions
not supports template-literals
not supports let
not supports const

We may take this higher once we confirm the browser support guarded by our “cut the mustard” checks
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3770 June 8, 2023 10:48 Inactive
@colinrotherham colinrotherham changed the base branch from main to module-script June 8, 2023 10:48
@colinrotherham colinrotherham linked an issue Jun 8, 2023 that may be closed by this pull request
2 tasks
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3770 June 8, 2023 10:53 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3770 June 8, 2023 14:01 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3770 June 8, 2023 14:06 Inactive
var $accordions = $scope.querySelectorAll('[data-module="govuk-accordion"]')
$accordions.forEach(function ($accordion) {
const $accordions = $scope.querySelectorAll('[data-module="govuk-accordion"]')
$accordions.forEach(($accordion) => {
Copy link
Contributor Author

@colinrotherham colinrotherham Jun 8, 2023

Choose a reason for hiding this comment

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

For all these arrow function changes I'm aware they're split into:

  1. Arrow functions used to preserve this as the component instance
  2. Arrow functions wrappers to avoid .bind(this) in event handlers
  3. Arrow functions simply for "code style" or consistency

We have plenty of event listeners and .forEach() loops (like this one) that never actually refer to this so we may consider it unnecessary to change them to arrow functions outside of component scope

Terser is going to convert all that it can to arrow functions anyway to save KBs

@36degrees 36degrees self-requested a review June 8, 2023 15:08
Base automatically changed from module-script to main June 8, 2023 16:37
@colinrotherham colinrotherham merged commit c11dfc0 into main Jun 8, 2023
@colinrotherham colinrotherham deleted the module-eslint-rules branch June 8, 2023 16:38
colinrotherham added a commit that referenced this pull request Jul 19, 2023
Upgrade ESLint configuration for ES2015 (without classes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Upgrade ESLint configuration
3 participants