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

Breaking change with v1.0.13 release #5852

Closed
codexp opened this issue Mar 20, 2020 · 17 comments
Closed

Breaking change with v1.0.13 release #5852

codexp opened this issue Mar 20, 2020 · 17 comments
Labels
question ❔ Issues where a question or problem is stated and a discussion is held to gather opinions.

Comments

@codexp
Copy link

codexp commented Mar 20, 2020

The last release breaks Webpack builds on node.js 8.*

eslint Plugin, has a dependency on mdn-browser-compat-data

        "eslint-plugin-compat": "^3.3.0",

It has been working fine until v1.0.13 release

yarn install

[2/4] Fetching packages...
error mdn-browser-compat-data@1.0.13: The engine "node" is incompatible with this module. Expected version ">=10.0.0". Got "8.17.0"
error Found incompatible module.

This is probably not compliant with semver?

Caused by: #5806

@queengooborg queengooborg added the question ❔ Issues where a question or problem is stated and a discussion is held to gather opinions. label Mar 20, 2020
@queengooborg
Copy link
Collaborator

Thanks for raising the issue! Upon initial thought, we figured that dropping support for a NodeJS release out of LTS wouldn't really be worth bumping the semver, and my initial research didn't lead to any definitive answers. However looking around, it seems that other packages bump the major version number when dropping support for a NodeJS version. So I guess, rather than 1.0.13, the release should have been 2.0.0. Pinging @Elchi3 on this one.

(P.S. you may want to consider updating your NodeJS version as Node v8 is out of LTS!)

@ddbeck
Copy link
Collaborator

ddbeck commented Mar 20, 2020

it seems that other packages bump the major version number when dropping support for a NodeJS version

Can you link to some examples of this?

@Elchi3
Copy link
Member

Elchi3 commented Mar 23, 2020

Hmm, should we then wait to bump the nodejs version?

As I understand #5806, we only did that in order to be a good citizen about our nodejs version... But if we're now actually breaking people's packages, we could probably back that out and release 1.0.14?

If we actually do release a 2.0.0 that would communicate a breaking change for sure, but do we really want to do that just to increase the (outdated) nodejs version?

@codexp
Copy link
Author

codexp commented Mar 23, 2020

In my case it breaks our builds and it may take a while until we will upgrade node.js.
There is probably no real reason to bump up node.js version, so older versions of node.js could still use this project without problems?

@ddbeck
Copy link
Collaborator

ddbeck commented Mar 23, 2020

The next patch release of BCD will fix this problem (see #5863). I'd expect this by Thursday (unless @Elchi3 opts to do a release sooner). Sorry about the broken release, @codexp, and thanks for your patience while we correct it.

In the meantime, I added a note to 1.0.13 warning of the problem in Node <10.0.0.

For future reference, Florian and I decided to defer bumping the engine version until we actually want to use some feature from Node v10. In other words, we're not going to bump the engine version until we actually commit some code that doesn't work in v8.

@shinepd
Copy link

shinepd commented Mar 23, 2020

Our builds are also broken and we don't have the ability to lock this package down as one of our transitive dependencies references this project with an ^.

We'd greatly appreciate a patch as soon as possible :)

https://github.com/amilajack/eslint-plugin-compat/blob/master/package.json#L65

@Elchi3
Copy link
Member

Elchi3 commented Mar 23, 2020

I've released 1.0.14! Let me know if this fixes the problems.

https://github.com/mdn/browser-compat-data/releases/tag/v1.0.14
https://www.npmjs.com/package/mdn-browser-compat-data

Thanks for your patience everyone! I've learned a lesson here that a project about compatibility should probably be very compatible with old or legacy versions as much as possible 😅

@Elchi3
Copy link
Member

Elchi3 commented Mar 24, 2020

@codexp @shinepd are you good now?

@shinepd
Copy link

shinepd commented Mar 24, 2020

We're good to go. Thank you for the quick turnaround!

@Elchi3 Elchi3 closed this as completed Mar 24, 2020
@codexp
Copy link
Author

codexp commented Mar 26, 2020

It works again, thank you very much!

@Elchi3
Copy link
Member

Elchi3 commented Apr 2, 2020

@shinepd @codexp just curious as more and more packages require Node 10 (prettier 2 and chalk in our case). How are you dealing with the situation?

@Elchi3
Copy link
Member

Elchi3 commented Apr 2, 2020

@shinepd @codexp see also #5931
I would appreciate your thoughts.

@codexp
Copy link
Author

codexp commented Apr 2, 2020

We decided to drop eslint-plugin-compat for now, but would reintroduce it some time later.
But if that's the way it goes (your deps drop support for deprecated nodejs also), probably the rollback is only temporary solution.

The path described in #5931 sounds nice.
That might keep this package functional for older nodejs, but only if you are able to freeze your deps before they require current nodejs version.

@queengooborg
Copy link
Collaborator

Considering that the packages follow semantic versioning rules (like we will remember to now), dependencies shouldn’t be too much of a problem for BCD v1.x, I’d say. We’ll only perform the package upgrades in v2.0. 😉

@shinepd
Copy link

shinepd commented Apr 2, 2020

@Elchi3 we're in the process of uplifting to Node 10 now so hopefully this won't be an issue for us for too long. We're definitely behind on getting to this update so I appreciate your willingness to work with us! I think the proposal in #5931 sounds great and agree with the semantic versioning proposal outlined by @vinyldarkscratch.

@Elchi3
Copy link
Member

Elchi3 commented Apr 3, 2020

Thanks for your feedback! I think we can wait a little longer before proceeding with the plan in #5931 as nodejs 10 isn't a hard requirement for us (at least as of right now). It would be just to update our devDependencies. So, probably, I will give this another month at which point most folks might react to this more like "oh bcd requires node 10 now. Well, glad we did update to that already".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question ❔ Issues where a question or problem is stated and a discussion is held to gather opinions.
Projects
None yet
Development

No branches or pull requests

5 participants