Skip to content

Remove polyfills from USWDS 3#4692

Merged
thisisdano merged 3 commits into
uswds:developfrom
aduth:aduth-rm-polyfill-3
Nov 6, 2024
Merged

Remove polyfills from USWDS 3#4692
thisisdano merged 3 commits into
uswds:developfrom
aduth:aduth-rm-polyfill-3

Conversation

@aduth

@aduth aduth commented May 16, 2022

Copy link
Copy Markdown
Contributor

Description

This pull request removes polyfills aimed at supporting Internet Explorer.

Additional information

Since Internet Explorer is no longer supported, polyfills which exist to add support for the browser should be removed.

Benefit for users: USWDS is as lean as it can be to avoid unnecessary excessive load times.
Benefit for developers: Fewer dependencies means faster install times and lower surface area risk for security vulnerabilities.

See related browser compatibility:

Performance comparison:

npm run build && du -k dist/js/uswds.min.js && gzip-size dist/js/uswds.min.js
  • Before: 92kb (24.9kb)
  • After: 84kb (23kb gzip)
  • Difference: -8kb / -8.7% (-1.9kb / -7.6% gzip)

Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

@aduth aduth mentioned this pull request May 16, 2022
4 tasks
@aduth

aduth commented May 16, 2022

Copy link
Copy Markdown
Contributor Author

On close inspection of our JavaScript bundles, I also noticed that USWDS packages a few additional polyfills transitively through the receptor package. I'm not sure how feasible it would be to remove those outside of an upstream patch, or moving away from the dependency altogether. However, it's worth noting for completeness' sake:

@aduth aduth force-pushed the aduth-rm-polyfill-3 branch from 070d593 to d561895 Compare November 17, 2022 19:48
@thisisdano thisisdano added Type: Feature Request New functionality wanted Affects: Performance Relates to the speed, size, or efficiency of our code Role: Dev Development/engineering skills needed Needs: Confirmation Need to confirm that this is an issue From: GSA labels Nov 2, 2023
@amyleadem amyleadem removed their assignment Nov 27, 2023
@brunerae brunerae requested a review from heymatthenry August 1, 2024 19:29
@brunerae brunerae added this to the uswds 3.9.0 milestone Aug 1, 2024
aduth added 2 commits August 2, 2024 08:27
Since Internet Explorer is no longer supported in the next major release (uswds#4538), polyfills which exist to add support for the browser should be removed.
@mahoneycm mahoneycm self-requested a review October 24, 2024 15:17
@mahoneycm

Copy link
Copy Markdown
Contributor

Accidental approval, flagging for re-review

@mahoneycm mahoneycm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me!

  • Methods meet baseline requirements
  • All references to polyfills removed
  • No perceived regressions
  • Code meets standards
  • No regression with Behavior
    • Spreading the props param prevents init issues
  • No regression with focus-trap
  • No perceived regressions when installed on site

@aduth can you resolve the merge conflicts when you have the chance?

@mahoneycm

Copy link
Copy Markdown
Contributor

Question: Would it be a good idea to remove references to the polyfills from our README ?

- **JavaScript** for components is located in `packages/[package]/src/index.js`. General JavaScript utilities and polyfills are located in the `uswds-core` package: `packages/uswds-core/src/js`

uswds/README.md

Line 515 in ae7129c

As of USWDS 3.0.0, we no longer officially support Internet Explorer 11 (IE11). We continued to include IE11 polyfills and prefixing for the first few releases in USWDS 3.x. We started to remove IE11-related polyfills in [USWDS 3.8.2](https://github.com/uswds/uswds/releases/tag/v3.8.2), and will continue to remove more of these polyfills in subsequent releases.

CC: @mejiaj @amyleadem

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this contribution, it's nice to see these finally getting removed!

Re: README

@mahoneycm that's a good note. We'll need to update the docs before the official release.

@mejiaj mejiaj requested a review from thisisdano October 30, 2024 14:15
@mejiaj mejiaj added Affects: Dependencies Relates to project dependencies Context: IE11 Related to IE11 support and removed Needs: Confirmation Need to confirm that this is an issue labels Nov 1, 2024

@thisisdano thisisdano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects: Dependencies Relates to project dependencies Affects: Performance Relates to the speed, size, or efficiency of our code Context: IE11 Related to IE11 support From: GSA Role: Dev Development/engineering skills needed Type: Feature Request New functionality wanted

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants