refactor: moved to compatible with React 17 #48
Conversation
semoal
force-pushed
the
migrated-to-testing-library
branch
from
a7dd861 to
bc59b63
Compare
|
@xile611 this pull request is ready, I can revert the lodash move if you don't feel "safe", i've tried this package in my projects and works good, but as you consider. Also... probably could be a good idea to drop React 15 support and upgrading react-transition-group to 4.x.x? |
semoal
force-pushed
the
migrated-to-testing-library
branch
from
bc59b63 to
ab5388d
Compare
semoal
force-pushed
the
migrated-to-testing-library
branch
from
ab5388d to
87fffc1
Compare
|
Any update on this? :) |
| "chai": "^4.1.2", | ||
| "chai-enzyme": "^0.8.0", | ||
| "chai-spies": "^0.7.1", | ||
| "core-js": "^3.9.1", |
There was a problem hiding this comment.
What's unused? Corejs is required by presetenv
There was a problem hiding this comment.
I gather it has to be explicitly enabled, and is disabled by default:
https://babeljs.io/docs/en/babel-preset-env#usebuiltins
(Which is good. Polyfill = application responsibility, not library)
There was a problem hiding this comment.
That's not 100% right, on recharts we've been shipping library compiled with env to be compatible with at least ie11 and last 2 versions.
There was a problem hiding this comment.
I noticed, and I think it's a bit unusual. The use of Webpack itself here is a bit unusual. It's a tool for bundling client js for applications. The underlying babel can/should be used by itself when we are creating libraries. Under the covers, webpack is just calling upon babel to do the transpile.
axios is another famous project for promulgating this approach, which has sharp edges.
How do large projects end up committing seemingly obvious errors?
Consider the timeline: in 2016 and earlier, we had very limited choices for putting together demos/docs. Naturally, webpack can do this for us, but some project(s) end up using webpack for the build too, which is really better left for babel alone to do.
Seen from an application developers' perspective: "Oh, guess I picked up another Promise implementation." or "Here goes another way to do Object.assign." The developer now needs to a) address bloat if there is a lot of polyfill cruft, b) worry about conflicts of interest in competing polyfill solutions.
The application developer needs to be in charge of their target environment(s). If they need a polyfill, they should add it... we cannot possibly imagine all the possible environments the code could be running in. And it's simple enough for the consumer to just add what they need.
There was a problem hiding this comment.
To get back on topic: @semoal
For this PR: I think it's fine if you need to add core-js (it was in recharts main repo at some point), but prefer to leave it out if possible. The creator is finally out of jail, which is good I guess.
I will write a more detailed post about this problem soon, as I haven't seen this problem described in any detail on the web. Sorry for the rant.
src/configUpdate.js
Outdated
| @@ -65,7 +64,8 @@ export default (from, to, easing, duration, render) => { | |||
| let update = () => null; | |||
|
|
|||
| const getCurrStyle = () => mapObject((key, val) => val.from, stepperStyle); | |||
| const shouldStopAnimation = () => !filter(stepperStyle, needContinue).length; | |||
| console.log(needContinue); | |||
| }, | ||
| "dependencies": { | ||
| "lodash": "~4.17.4", |
Checks:
✅ npm run test
✅ npm run lint
✅ npm run demo -> 4 examples are correctly rendered
Questions:
❓ Should I create a Github Action file? Looks more interesting than Travis, that looks isn't working on pull requests. I have already one ready if you're comfortable with github actions i push one
Master branch:
This branch: