Update to babel 6 #78 #81
Conversation
lecstor
commented
Dec 18, 2015
lecstor
commented
- tests pass with React 13 & 14
- npm run travis produces coverage
- npm link works
|
hrmm bugga. iojs-v3.3 |
|
@lecstor |
| "babel-preset-react": "^6.3.13", | ||
| "babel-preset-stage-0": "^6.3.13", | ||
| "babel-register": "^6.3.13", | ||
| "babel-runtime": "^6.3.19", |
There was a problem hiding this comment.
It seems very unfortunate to require babel-runtime in a reusable module. Why is this necessary?
There was a problem hiding this comment.
thanks, you are correct, it is not required and will be removed.
|
|
|
@lecstor the single use could be replaced with |
| @@ -47,20 +47,25 @@ | |||
| "license": "MIT", | |||
| "dependencies": { | |||
| "cheerio": "^0.19.0", | |||
| "object-assign": "^4.0.1", | |||
There was a problem hiding this comment.
please use object.assign instead of object-assign - it's more spec-compliant.
|
sorry about that. I haven't needed to use it before (obviously 8). |
|
Since this isn't a top-level app, only a reusable module, modifying the global environment in any way is unacceptable - so please don't use the |
|
done. I think. I'm not entirely sure if that is the correct usage. Reading the source it seems it should be fine, but the readme doesn't mention that usage style at all. The examples all call shim or getPolyfill, and assign the result to shimmedAssign mostly, but use |
|
@lecstor that looks great, thanks! The readme on https://www.npmjs.com/package/object.assign has the examples under "most common usage". "getPolyfill" doesn't mutate the environment (shims do that), it just returns the native method when it complies with the spec. |
|
excellent, thank you @ljharb |
| isDOMComponent, | ||
| isCompositeComponent, | ||
| isCompositeComponentWithType, | ||
| isElement, | ||
| findDOMNode, | ||
| } from './react-compat'; | ||
| } = reactCompat; |
There was a problem hiding this comment.
was this changed for a reason?
There was a problem hiding this comment.
babel 5 incorrectly allows inline destructuring on a default import - the spec does not, and babel 6 changed the module interop so that this doesn't wrongly work.
There was a problem hiding this comment.
this makes me sad :/
There was a problem hiding this comment.
Why don't we fix this in react-compat.js?
Just export all these individually.
There was a problem hiding this comment.
👍 i agree that would be better
There was a problem hiding this comment.
PR updated accordingly.
lecstor
force-pushed
the
master
branch
2 times, most recently
from
e50556f
to
5f1a043
Compare
|
Hey @lecstor, we are wanting to merge this now.... Can you rebase and squash your commits? Thanks! |
lecstor
force-pushed
the
master
branch
2 times, most recently
from
9f9f88a
to
67451ef
Compare
- tests pass with React 13 & 14 - npm run travis produces coverage - npm link works - use airbnb Babel6 preset - modify react-compat to export functions individually - update test commands to use babel-core
|
Hi @lelandrichardson, I had to make a final change to update the test commands to use |
Update to babel 6 #78
