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

chore: set browsers target to defaults #286

Merged
merged 2 commits into from
May 12, 2024

Conversation

VIKTORVAV99
Copy link
Contributor

@VIKTORVAV99 VIKTORVAV99 commented May 11, 2024

Technically a breaking change but I believe it makes sense to only support es6 supported browsers which get's rid of a lot of the polyfills that babel need to insert which is inflating the bundle a lot. It also allows babel to use object deconstruction internally in the transformers and other newer syntax so further reduce the bundle.

The minified file in the top level of the repo went from 7 540 byte to 6 341 byte. A reduction of ~16%.

ES6 is supported by 97.1% of the browsers right now: https://browsersl.ist/#q=%22browserslist%22%3A+%5B%0A++++++++%22fully+supports+es6%22%0A++++++%5D

I didn't see any reference to which browsers or JS versions that are supported so I don't believe this requires any documentation updates but please let me know if it does.

PS: If you prefer this as a option inside the .babelrc file instead I'd be happy to change it but in general I think it's best to set it in the package.json so all tooling picks it up.

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

Checklist (for documentation change)

  • only relevant documentation part is changed (make a diff before you submit the PR)
  • motivation/reason is provided
  • commit message and code follows the Developer's Certification of Origin

@VIKTORVAV99
Copy link
Contributor Author

VIKTORVAV99 commented May 11, 2024

I would also be happy to change this to what matches the main i18n package but that requires installing @babel/preset-react as well. (Which shouldn’t be needed in either btw as there is no jsx in the source code.)

Might make sense to have them all unified though.

@adrai
Copy link
Member

adrai commented May 12, 2024

should be in line with react-i18next

@adrai
Copy link
Member

adrai commented May 12, 2024

at the end it's: "1kb of size reduction" vs. "dropping support for a lot of older environments"... is this really a huge benefit?

@VIKTORVAV99
Copy link
Contributor Author

VIKTORVAV99 commented May 12, 2024

at the end it's: "1kb of size reduction" vs. "dropping support for a lot of older environments"... is this really a huge benefit?

Considering we have to load i18next and all plugins for it in our index bundle making that as small as possible is very much something that would benefit us.

There is also a small environmental impact to this as it would save just us tens of gigabytes in lowered data transfers per year from just this simple change.

I haven't checked how it would look with the react-i18next config in terms of numbers but I'd say dropping support for 2.9% of the users with this current config is not really a huge deal. Especially since the consumers of this package don't have to update if they really need it. For example 100% of our own user base at @electricitymaps would be supported and there would only be upsides to this for us.

@adrai
Copy link
Member

adrai commented May 12, 2024

i think it's relatively simple... we can update it... but if there will be complaints... we will consider to revert it

@VIKTORVAV99
Copy link
Contributor Author

I'll update this PR to match the react-i18next config as soon as I have some time then.

@VIKTORVAV99
Copy link
Contributor Author

I have matched the react-i18next config where it made sense, @babel/plugin-transform-runtime is already included in the @babel/preset-env and @babel/preset-react is not needed as this package don't have any jsx source code.

The size of the generated minified code is now 6 370 bytes. Likely cause it is using const and let as the source code did and not converting everything to var.

@VIKTORVAV99 VIKTORVAV99 changed the title chore: add browserslist config "fully support es6" chore: set browsers target to defaults May 12, 2024
@VIKTORVAV99
Copy link
Contributor Author

BTW: Once this is merged I plan on opening a follow up PR that changes code to take advantage of these changes.

@adrai adrai merged commit 5b046ce into i18next:master May 12, 2024
2 checks passed
@VIKTORVAV99 VIKTORVAV99 deleted the browserslist_config branch May 12, 2024 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants