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

[WIP] Update dependencies #640

Closed
wants to merge 6 commits into from
Closed

[WIP] Update dependencies #640

wants to merge 6 commits into from

Conversation

gr2m
Copy link
Collaborator

@gr2m gr2m commented May 18, 2019

Follow up #626 (comment)

The tests are passing, but the build fails. I don’t use babel or rollup myself, any idea what the error could mean?

$ npm run build

> node-fetch@2.6.0 build /node-fetch
> cross-env BABEL_ENV=rollup rollup -c


src/index.js → lib/index.js, lib/index.es.js, lib/index.mjs...
[object Object]
    at Object.isRestProperty (/node-fetch/node_modules/@babel/types/lib/validators/generated/index.js:4288:11)
    at DestructuringTransformer.pushObjectPattern (/node-fetch/node_modules/babel-plugin-transform-es2015-destructuring/lib/index.js:197:15)
    at DestructuringTransformer.push (/node-fetch/node_modules/babel-plugin-transform-es2015-destructuring/lib/index.js:108:14)
    at DestructuringTransformer.init (/node-fetch/node_modules/babel-plugin-transform-es2015-destructuring/lib/index.js:317:12)
    at PluginPass.VariableDeclaration (/node-fetch/node_modules/babel-plugin-transform-es2015-destructuring/lib/index.js:468:27)
    at newFn (/node-fetch/node_modules/@babel/traverse/lib/visitors.js:193:21)
    at NodePath._call (/node-fetch/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (/node-fetch/node_modules/@babel/traverse/lib/path/context.js:40:17)
    at NodePath.visit (/node-fetch/node_modules/@babel/traverse/lib/path/context.js:88:12)
    at TraversalContext.visitQueue (/node-fetch/node_modules/@babel/traverse/lib/context.js:118:16)

# ... more of the same

(!) Plugin at position 2 plugin: The transformBundle hook used by plugin at position 2 is deprecated. The renderChunk hook should be used instead.
created lib/index.js, lib/index.es.js, lib/index.mjs in 1.2s

@gr2m gr2m changed the title Update dependencies [WIP] Update dependencies May 18, 2019
@bitinn
Copy link
Collaborator

bitinn commented May 18, 2019

No idea yet, haven't used babel lately.

People in the know, feel free to suggest a fix.

@dmitriz
Copy link

dmitriz commented Jun 7, 2019

@bitinn

No idea yet, haven't used babel lately.
People in the know, feel free to suggest a fix.

It seems like using babel is holding up the development
with people less likely to contribute
which is especially concerning in view of
#567

My question -- is there any tangible benefit here to use babel,
strong enough to compensate for the added complexity?

From looking over the code, I can only see it used for the ES6 imports.
Replacing those with CommonJS would seemingly eliminate the need to use babel.
This is not to debate the general benefits of the ES6 imports,
rather the more practical question - to what extent these really matter for this project?

@bitinn
Copy link
Collaborator

bitinn commented Jun 7, 2019

#643 I plan to drop it or update it, but have no time to investigate this issue yet.

@Richienb
Copy link
Member

Richienb commented Sep 8, 2019

During the development of 3.x, we've updated all the dependencies, primarily in 155ef26.

@Richienb Richienb closed this Sep 8, 2019
@gr2m gr2m deleted the update-dependencies branch June 11, 2020 22:40
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

4 participants