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

changing the node engine to >=8.6.0 #250

Merged
merged 1 commit into from
Jan 30, 2019
Merged

Conversation

blackdynamo
Copy link
Contributor

Updating the docs and the package.json to so the minimum node version is displayed to potential users of the module.

Ran into some issues:

node_modules/kafkajs/src/index.js:57
          retry: { ...cluster.retry, ...retry },
                   ^^^

    SyntaxError: Unexpected token ...

      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/script_transformer.js:403:17)
      at Object.<anonymous> (node_modules/kafkajs/index.js:1:42)

Because the index.js uses object spread syntax without transpiling, users will get this error. Object spread was added to node without a flag in v8.6.0.
https://node.green/#ES2018-features-object-rest-spread-properties

@tulios
Copy link
Owner

tulios commented Jan 30, 2019

Thanks @blackdynamo, I started migrating Object.assign to spread but failed to realize the requirement. Since you are here, how much of a deal is for your company/project to update the node version? Just giving you some context, we are planning to get rid of our last dependency by supporting node 10 BigInt, but we haven't planned the steps yet.

Thanks again!

@tulios tulios merged commit cbfed51 into tulios:master Jan 30, 2019
@Nevon Nevon mentioned this pull request Jan 30, 2019
3 tasks
@blackdynamo
Copy link
Contributor Author

@tulios Sorry for the late reply, I just realized you asked a question!

To answer your question, we are going through the slow process of bumping to node 10, but I would hesitate to release this module locking to >= node 10 so quickly. It restricts the reach of this module. This is why tools like babel exist to allow you to take advantage of the newer features of ES but still support legacy versions of node/browsers etc.

Of course this is just my opinion and you have your reasons for not using babel as of yet. I think it's more receptive to support the current two versions of node at least. That gives people time to update their services that might use this module. As an example we have 20 services that use this module now, with varying versions of dependencies. Some of those dependencies don't allow use to move to node 10 without updating them. Some of the apis of those dependencies changed so that requires a lot of dev/qa effort to move up, even though it's scheduled for the year.

Just some food for thought :)

@Nevon
Copy link
Collaborator

Nevon commented Feb 13, 2019

This is great input for #251. My current thinking is that we'll keep supporting 8.x.x until the end of the year (basically when the support period from node itself ends), but we haven't committed to anything yet.

@tulios
Copy link
Owner

tulios commented Feb 13, 2019

Hi @blackdynamo, thanks for the information, this helps us shape the future. We praise stability and don't have any concrete plans to deprecate node 8. We want to remove all dependencies and node 10 should facilitate that, but we are working on a strategy for that.

Thanks again.

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.

3 participants