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

Remove IE8-10 JavaScript polyfills, helpers, config #3570

Merged
merged 10 commits into from
May 4, 2023
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented May 2, 2023

This PR unblocks #3279 by removing Rollup { legacy: true } so we can pick up:

Similar to #3559 this PR also tackles IE8-10 JavaScript things:

  1. Remove the nodeListForEach helper function #3466
  2. Remove polyfills for IE #2506 (but excludes those for IE11 for now)
  3. Removes IE8 legacy support for Rollup and Terser

@colinrotherham colinrotherham self-assigned this May 2, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 2, 2023 16:17 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 2, 2023 16:27 Inactive
@colinrotherham colinrotherham changed the title Remove IE8-related JavaScript packages and settings Remove IE8-10 JavaScript packages and settings May 3, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 3, 2023 08:40 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 3, 2023 08:53 Inactive
Comment on lines -11 to -30
/**
* TODO: Ideally this would be a NodeList.prototype.forEach polyfill
* This seems to fail in IE8, requires more investigation.
* See: https://github.com/imagitama/nodelist-foreach-polyfill
*
* @deprecated Will be made private in v5.0
* @template {Node} ElementType
* @param {NodeListOf<ElementType>} nodes - NodeList from querySelectorAll()
* @param {(value: ElementType, index: number, nodes: NodeListOf<ElementType>) => void} callback - Callback function to run for each node
* @returns {void}
*/
export function nodeListForEach (nodes, callback) {
if (window.NodeList.prototype.forEach) {
return nodes.forEach(callback)
}
for (var i = 0; i < nodes.length; i++) {
callback.call(window, nodes[i], i, nodes)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Animated GIF of a young Asian girl saying "The evil is defeated."

@colinrotherham colinrotherham changed the title Remove IE8-10 JavaScript packages and settings Remove IE8-10 JavaScript polyfills, helpers, config May 3, 2023
@colinrotherham colinrotherham mentioned this pull request May 3, 2023
5 tasks
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 3, 2023 12:27 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 3, 2023 12:50 Inactive
@colinrotherham colinrotherham changed the base branch from review-app-user to remove-ie8-css May 3, 2023 12:50
@colinrotherham
Copy link
Contributor Author

@querkmachine I've rebased on top of #3559 to avoid Review app conditional comment conflicts etc

Think we're looking good?

@@ -41,7 +41,7 @@ module.exports = {
extends: [
'plugin:@typescript-eslint/recommended',
'plugin:@typescript-eslint/recommended-requiring-type-checking',
'plugin:es-x/restrict-to-es3'
'plugin:es-x/restrict-to-es5'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can raise this even higher once IE11 is prevented from loading JS
https://eslint-community.github.io/eslint-plugin-es-x/#presets

Copy link
Member

@querkmachine querkmachine left a comment

Choose a reason for hiding this comment

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

I haven't gone through it with the finest of combs, but what's here works and looks proper to me. On the off chance we missed anything, we have ample time to fix it up.

Base automatically changed from remove-ie8-css to review-app-user May 3, 2023 13:04
@colinrotherham colinrotherham changed the base branch from review-app-user to remove-ie8-css May 3, 2023 15:24
@colinrotherham colinrotherham requested a review from a team as a code owner May 3, 2023 16:38
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 3, 2023 16:43 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 3, 2023 16:59 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3570 May 4, 2023 11:35 Inactive
Base automatically changed from remove-ie8-css to main May 4, 2023 14:04
@colinrotherham colinrotherham merged commit a7f1e57 into main May 4, 2023
@colinrotherham colinrotherham deleted the remove-ie8-js branch May 4, 2023 14:15
romaricpascal pushed a commit that referenced this pull request May 18, 2023
Remove IE8-10 JavaScript polyfills, helpers, config
@romaricpascal romaricpascal mentioned this pull request Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants