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

Bump to babel 7 and switch from es2015 to babel-preset-env #1066

Closed
adrienharnay opened this issue Jan 30, 2019 · 7 comments
Closed

Bump to babel 7 and switch from es2015 to babel-preset-env #1066

adrienharnay opened this issue Jan 30, 2019 · 7 comments

Comments

@adrienharnay
Copy link

Hello,

I figured that it would be great for this project to have a browserlists file and to use @babel/preset-env to ensure support for older browsers is never compromised.

Would you be interested in me submitting a pull request?

Cheers

@kossnocorp
Copy link
Member

That's a good question. We intentionally didn't introduce modern JS features because you never know what Babel would generate or which polyfills it would include and how this will affect the build size. That's a pandora box that I would rather not open. But I understand the concern. Can it be solved using ESLint?

@adrienharnay
Copy link
Author

adrienharnay commented Jan 30, 2019

I don't think this so...

What do you think of this strategy:

  • upgrade to Babel 7
  • implement @babel/preset-env with a browserslist file
  • compare old build size to new build size
  • implement https://github.com/siddharthkp/bundlesize to ensure the size of the build never increases passed a certain ceiling (to be defined)

I've implemented this on a few projects so I could do a PR if you think this could work for date-fns 🙂

PS: it would allow maintainers to use new JS features, which are sometimes more readable/performant that their old alternatives

@kossnocorp
Copy link
Member

That will add a lot of additional overhead to the maintenance process. I'm not concerned with the overall bundle size but rather with the size of individual functions. A new locale will always add a lot of code, and it's okay while even 0.1Kb increase in a single function could be a problem.

Quick googling showed that the problem could be solved with eslint-plugin-compat that reads the browserslist file, so I'm ok with upgrading Babel and adding browserlist but not @babel/preset-env. Also, we already use size-limit for checking bundle size but do it manually when necessary.

@adrienharnay
Copy link
Author

Ah gotcha! Do you want me to file the PR for Babel 7 and eslint-plugin-compat?

@kossnocorp
Copy link
Member

That would be helpful, thanks! 🙏

@nanomosfet
Copy link
Contributor

@adrienharnay Do you have any examples of comparing build sizes? I am pretty interested in seeing how its done.

@adrienharnay
Copy link
Author

Hey, sorry for the delay, the past weeks have been busy. Well I'm using https://github.com/siddharthkp/bundlesize along with its Github build status integration, I have a config in my package.json to specify the thresholds and it compares the size of the build on my PRs to the size on master. I don't have any concrete example to show as the repositories are private, but it's fairly easy to set up using the docs (took me 10min). Tell me if I can provide more info on this :)

codinsonn pushed a commit to codinsonn/date-fns that referenced this issue Jun 9, 2019
…#875; closes date-fns#1066)

* Setup Jest
* Update Babel and Power Assert
* Make test karma suite work
* Fix multiple it used instead of describe
* Fix accidentally disabled tests
* Upgrade Sinon
* Add Node.js suite to Travis CI
* Remove node_modules from .flowconfig ignore
* Remove console.log from a test
* Use regexp for exception assertion to make it work in any Node.js
* Disable Sauce Connect because it gives me headaches
codinsonn pushed a commit to codinsonn/date-fns that referenced this issue Jun 10, 2019
…#875; closes date-fns#1066)

* Setup Jest
* Update Babel and Power Assert
* Make test karma suite work
* Fix multiple it used instead of describe
* Fix accidentally disabled tests
* Upgrade Sinon
* Add Node.js suite to Travis CI
* Remove node_modules from .flowconfig ignore
* Remove console.log from a test
* Use regexp for exception assertion to make it work in any Node.js
* Disable Sauce Connect because it gives me headaches
elmomalmo pushed a commit to elmomalmo/date-fns that referenced this issue Jul 12, 2019
…#875; closes date-fns#1066)

* Setup Jest
* Update Babel and Power Assert
* Make test karma suite work
* Fix multiple it used instead of describe
* Fix accidentally disabled tests
* Upgrade Sinon
* Add Node.js suite to Travis CI
* Remove node_modules from .flowconfig ignore
* Remove console.log from a test
* Use regexp for exception assertion to make it work in any Node.js
* Disable Sauce Connect because it gives me headaches
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants