Skip to content
This repository was archived by the owner on May 10, 2021. It is now read-only.

Add support for Node 10.17.0. #84

Merged
merged 1 commit into from
Nov 16, 2020
Merged

Add support for Node 10.17.0. #84

merged 1 commit into from
Nov 16, 2020

Conversation

ehmicky
Copy link
Contributor

@ehmicky ehmicky commented Nov 16, 2020

Node 10.17.0 could not run tests due to a test helpers using a JavaScript feature added in Node 12 (instance fields).
This PR fixes this.
It also runs tests on Node 10.17.0 and adds engines.node to declare the supported Node.js versions.

@ehmicky ehmicky added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Nov 16, 2020
@ehmicky ehmicky self-assigned this Nov 16, 2020
@@ -19,9 +19,6 @@ const { NEXT_VERSION } = require("./nextVersion");
// run next-on-netlify.

class NextAppBuilder {
// Name of the app to build. This determines the build path.
__appName = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using instance fields is not supported in Node 10.

__appName is used below in path.join() which expects a specific value to have been configured with forTest(). So this can be safely removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

things i wouldve probably never found: this

@@ -26,6 +26,6 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
- name: Install dependencies
run: npm ci
run: npm install
Copy link
Contributor Author

@ehmicky ehmicky Nov 16, 2020

Choose a reason for hiding this comment

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

npm ci is not support in the older npm version which comes with Node 10.17.0.

@ehmicky ehmicky changed the title Add support for Node 10 Add support for Node 10.17.0. Nov 16, 2020
Copy link
Contributor

@lindsaylevine lindsaylevine left a comment

Choose a reason for hiding this comment

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

👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏

@@ -19,9 +19,6 @@ const { NEXT_VERSION } = require("./nextVersion");
// run next-on-netlify.

class NextAppBuilder {
// Name of the app to build. This determines the build path.
__appName = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

things i wouldve probably never found: this

@ehmicky ehmicky merged commit a9dade3 into main Nov 16, 2020
@ehmicky ehmicky deleted the chore/add-node-10 branch November 16, 2020 19:32
lindsaylevine added a commit that referenced this pull request Nov 17, 2020
- Upgrade version range of `next` ([#90](#90))
- Configurable functionsDir and publishDir (via exported func only) ([#89](#89))
- Support for Node 10.17.0 ([#84](#84))
- CI tests ([#83](#83))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants