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

Drop Node.js required version from 8.15.0 to 8.7.0 #4032

Merged
merged 4 commits into from Apr 16, 2019
Merged

Conversation

ntwb
Copy link
Member

@ntwb ntwb commented Apr 15, 2019

Via #4000 (comment)

As AWS Labda currenty only supports Node.js 8.10, let's drop the required version down to 8.0.0

We can add this to the next minor release 10.0.1

@hudochenkov
Copy link
Member

What if we, or our dependency, uses a feature introduced in 8.3.0 for example?

@jeddy3
Copy link
Member

jeddy3 commented Apr 15, 2019

What if we, or our dependency, uses a feature introduced in 8.3.0 for example?

I tested locally with 8.0.0 and stylelint crashes out with

/Users/jeddy3/Projects/stylelint/node_modules/micromatch/index.js:44
    let isMatch = picomatch(String(patterns[i]), { ...options, onResult }, true);
                                                   ^^^
SyntaxError: Unexpected token ...

This is because micromatch uses the object spread operator which is behind a flag until 8.3.0.

micromatch has >=8 in its package.json, which gives me the impression that this engine field is a bit problematic.

According to node.green the following versions added new features:

  • 8.7.0
  • 8.3.0

Even though stylelint works with 8.3.0 I suggest we go with 8.7.0 as that's the lowest feature-complete version of 8 (and it's compatible with AWS). This seems to me to be the best compromise.

@jeddy3 jeddy3 mentioned this pull request Apr 15, 2019
4 tasks
@ntwb
Copy link
Member Author

ntwb commented Apr 15, 2019

Thanks, makes sense, have bumped it to 8.7.0

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jeddy3 jeddy3 changed the title Drop Node.js required version from 8.15.0 to 8.0.0 Drop Node.js required version from 8.15.0 to 8.7.0 Apr 15, 2019
@jeddy3
Copy link
Member

jeddy3 commented Apr 16, 2019

@hudochenkov Did you have any more thoughts on this? I can put out a patch release this morning if this goes in.

Copy link
Member

@hudochenkov hudochenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jeddy3 jeddy3 merged commit 1d204c2 into master Apr 16, 2019
@jeddy3 jeddy3 deleted the drop-node-version branch April 16, 2019 08:34
@jeddy3
Copy link
Member

jeddy3 commented Apr 16, 2019

  • Fixed: minimum Node.js engine reduced to 8.7.0 (#4032).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants