Drop core-js in favour of custom fallbacks #1725
Conversation
Ref atlassian#1629 (comment) I forbid using es6+ methods I added eslint-plugin-es6. It does not forbid isInteger but I send PR nkt/eslint-plugin-es5#37.
.size-snapshot.json
Outdated
| "bundled": 321489, | ||
| "minified": 115006, | ||
| "gzipped": 34084 | ||
| "bundled": 307334, |
package.json
Outdated
| @@ -56,7 +56,7 @@ | |||
| "prepublishOnly": "yarn build" | |||
| }, | |||
| "dependencies": { | |||
| "@babel/runtime-corejs2": "^7.6.3", | |||
| "@babel/runtime": "^7.6.3", | |||
There was a problem hiding this comment.
thanks for sorting this!
| // @flow | ||
| // https://allyjs.io/tutorials/hiding-elements.html | ||
| // Element is visually hidden but is readable by screen readers | ||
| const assignVisuallyHidden = (style: Object) => { |
There was a problem hiding this comment.
I prefer my functions not to mutate. Are we avoiding Object.assign?
There was a problem hiding this comment.
Yes. Object.assign need to be polyfilled. Though in this case much easier to use mutation. I think this is ok for this particular function. object assign ponyfill would mutate too.
There was a problem hiding this comment.
We use spread {...values, ...otherValues} quite a bit in the library. Wouldn't that be using Object.assign?
There was a problem hiding this comment.
Spread uses extends helpers which comes from @babel/runtime package
There was a problem hiding this comment.
But it won’t use that if you use object.assign. Can we import extends from Babel runtime and use that for the assign? 🤔
There was a problem hiding this comment.
My ideal is to not have a function modify the passed in object but rather to return a new object (or to do the Object.assign in place) But I am okay to not get hung up on it and merge soon
| @@ -70,6 +70,7 @@ | |||
| "@babel/core": "^7.6.4", | |||
| "@babel/plugin-proposal-class-properties": "^7.5.5", | |||
| "@babel/plugin-transform-modules-commonjs": "^7.6.0", | |||
| "@babel/plugin-transform-object-assign": "^7.8.3", | |||
There was a problem hiding this comment.
just to confirm - this won't polyfill, it is just ponyfill?
There was a problem hiding this comment.
|
You will be able to get rid from eslint suppression in new release of es5 plugin |
|
Thanks for this @TrySound. I will aim to release this on monday |
|
Are you guys still planning on a release with this soon? |
|
@davidchenfitzgerald Yes, after fixing a few minor issues |
Ref #1629 (comment)
I forbid using es6+ methods I added eslint-plugin-es6. It does not
forbid isInteger but I send PR nkt/eslint-plugin-es5#37.
Looks like we saved 2kb gzipped. And one less commonjs dependency!