Skip to content

Commit

Permalink
update changelog, travis, and engines versions
Browse files Browse the repository at this point in the history
  • Loading branch information
@isaacs
isaacs committed Dec 14, 2019
1 parent 533ed12 commit d61f828
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 2 deletions.
4 changes: 2 additions & 2 deletions .travis.yml
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
language: node_js

node_js:
- 11
- node
- 12
- 10
- 8

os:
- linux
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# changes log

## 7.0.0

* Refactor module into separate files for better tree-shaking
* Drop support for very old node versions, use const/let, `=>` functions,
and classes.

## 6.3.0

* Expose the token enum on the exports
Expand Down
3 changes: 3 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,8 @@
"tap": {
"check-coverage": true,
"coverage-map": "map.js"
},
"engines": {
"node": ">=10"

This comment has been minimized.

Sign in to view

Copy link
@eventualbuddha

eventualbuddha Dec 17, 2019

The irony that a breaking change happened in a minor release (7.1.0) with this package is too much.

This comment has been minimized.

Sign in to view

Copy link
@zloirock

zloirock Dec 17, 2019

Why Node 10? "const/let, => functions, and classes" available even in 6.4.

This comment has been minimized.

Sign in to view

Copy link
@isaacs

isaacs Dec 17, 2019

Author Contributor

@eventualbuddha The breaking change happened in v7.0.0.

@zloirock Node 8 hits EOL for even major security fixes in 2 weeks. Node 6 has been EOL for some time now. This module might work on it, but our team doesn't support or guarantee it will work on those node versions. Printing a warning is not a breaking change, it's informational.

This comment has been minimized.

Sign in to view

Copy link
@zloirock

zloirock Dec 17, 2019 •
edited
Loading

@isaacs

This module might work on it, but our team doesn't support or guarantee it will work on those node versions. Printing a warning is not a breaking change, it's informational.

I don't use Yarn, but seems for Yarn users it's a breaking change like here zloirock/core-js#669

Node 8 hits EOL for even major security fixes in 2 weeks. Node 6 has been EOL for some time now.

Sure, but some modules (like core-js-compat) already were updated to semver@^7.0, but they should support Node 8 since it's a breaking change for them.

This comment has been minimized.

Sign in to view

Copy link
@eventualbuddha

eventualbuddha Dec 17, 2019

Ah, I didn't realize that npm and yarn differed in that regard. So yeah, I mostly use yarn and from yarn's perspective, 7.1.0 was the breaking change and not 7.0.0:

❯ NODE_VERSION=8.16.2 yarn add semver@7.0.0 
yarn add v1.21.1
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...

success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ semver@7.0.0
info All dependencies
└─ semver@7.0.0
Done in 0.44s.

❯ NODE_VERSION=8.16.2 yarn add semver@7.1.0
yarn add v1.21.1
warning package.json: No license field
warning No license field
[1/4] Resolving packages...
[2/4] Fetching packages...
error semver@7.1.0: The engine "node" is incompatible with this module. Expected version ">=10". Got "8.16.2"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.

This comment has been minimized.

Sign in to view

Copy link
@isaacs

isaacs Dec 17, 2019

Author Contributor

Seems like something you should take up with the yarn maintainers.

This comment has been minimized.

Sign in to view

Copy link
@vladshcherbin

vladshcherbin Jan 26, 2020

Can't believe node version drops can happen in minor versions 🤦‍♂

Latest Jest version already has an issue with this, expect more to come.

This comment has been minimized.

Sign in to view

Copy link
@tdreyno

tdreyno Jan 29, 2020

So, I would still like to support Node 8 for a little while for my users. Node 8 is still the stable release for Google Cloud Functions. This change makes it impossible for me to have jest tests and support Google Cloud Functions users.

This comment has been minimized.

Sign in to view

Copy link
@isaacs

isaacs Jan 29, 2020

Author Contributor

Then use SemVer v6, or install with --no-engine-strict.

This comment has been minimized.

Sign in to view

Copy link
@isaacs

isaacs Jan 29, 2020

Author Contributor

By the way, the engines block in package.json is a gift here. SemVer v7 was liable to break node 8 users regardless of what the engines block says. The engines object is a way for package authors like myself to indicate what versions we intend for this module to support, and what platform versions may be broken without warning if they use this code. It's a kind and gracious thing to do, and I absolutely reject the notion that dropping support for an EOL platform is a "breaking change".

If you are using EOL versions of software, you're on your own when it comes to support. That means:

  • No security updates.
  • No bugfixes.
  • Breakage from any dependency updates within non-semver-major version bumps (because the API didn't change, the platform required did.)

Lock down your deps. Pin your versions at least, or better yet, check node_modules into source control. Sometimes it's what you gotta do, and that's fine. Just please don't blame the road-builders when you wander off into the jungle. The LTS schedules are posted years in advance, it's not like this should be a surprise. If you pull in an update that doesn't work on your EOL platform, I'm sorry that happened to you, but it's an error downstream from the package publisher.

That being said, I do typically bump major when dropping support for a platform version. That's what actually happened in this case, just neglected to update the advertisement of such. That bug was fixed within the 7.x line. Yarn refusing to install it is not a SemVer API change, it's a Yarn issue. If you don't like that behavior, I do know of another package manager you can use :)

This comment has been minimized.

Sign in to view

Copy link
@Borewit

Borewit May 24, 2020

The engine version should indicate engine compatibility (the engine it works with). As long there is no breaking change with version 8, it should not be set to 10. That is what the semantics of that fields is.

Created bug: #327
}
}

0 comments on commit d61f828

Please sign in to comment.