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

Expecting Node 9 #3

Closed
MarcL opened this issue Mar 2, 2018 · 9 comments
Closed

Expecting Node 9 #3

MarcL opened this issue Mar 2, 2018 · 9 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@MarcL
Copy link

MarcL commented Mar 2, 2018

In package.json you're setting engines as >=9.0.0 and we're currently running node 6 LTS (6.12.0). We will be switching to Node 8 LTS at some point soon but haven't yet.

Does it need Node 9 or can you drop it back and/or transpile?

@neilstuartcraig
Copy link
Owner

Hi @MarcL
Ah good point, i forgot about that...It does (or at least did) genuinely need node 9 currently but i think i can transpile easily enough. The lib file is already transpiled so i should just be able to reused that mechanism and add the config for Travis.

I'd be really interested to know your thoughts specifically on the tests (i am very far from an expert on testing) and also the docs - e.g. would a "how it works" section be useful (will people care?). All thoughts gratefully received!

Cheers

@neilstuartcraig neilstuartcraig self-assigned this Mar 2, 2018
@neilstuartcraig neilstuartcraig added the enhancement New feature or request label Mar 2, 2018
@neilstuartcraig
Copy link
Owner

Ah OK, so the issue i hit first off with node 6 is that ava doesn't like the async in the test definition:

/home/travis/build/neilstuartcraig/git-off-my-land/test/formatOutput-tests.js:13
(0, _ava2.default)("Correct operation, valid, populated inputs, unsorted -> sorted", async t => {
                                                                                     ^^^^^
SyntaxError: missing ) after argument list
...

Maybe it needs an extra bit of ava config or a plugin or something. I'll try to find out what.

@neilstuartcraig
Copy link
Owner

Ah OK, i think i'm over-complicating it. I think supporting node < 9 means transpiling the tests too (?) - i am assuming Ava doesn't transpile the tests.

@neilstuartcraig
Copy link
Owner

Hmm, i've tried (with a tired brain) a few things on branch goml-3 to no avail...
Seems Ava does transpile code as per babel (i'm telling it to via "inherit").
Will do better research and try again once i'm back from leave.

@neilstuartcraig neilstuartcraig added the help wanted Extra attention is needed label Mar 2, 2018
@neilstuartcraig
Copy link
Owner

Clarification: Node < 8 cannot run unit tests but the pre-commit hook works fine (thanks to Babel) on Node 6, 7, 8, 9 at least.

@neilstuartcraig
Copy link
Owner

Ensure to update the readme to remove the note about this if it gets fixed

@MarcL
Copy link
Author

MarcL commented May 14, 2018

Just tried installing with Node 6.14.1 and it expects Node 8 for its post install step for fs.copyFileSync I think:

❯ npm install --save-dev git-off-my-land

> git-off-my-land@2.2.5 postinstall /Users/littlm07/dev/id-account/node_modules/git-off-my-land
> ./scripts/post-install.js

TypeError: fs.copyFileSync is not a function
    at Object.<anonymous> (/Users/littlm07/dev/id-account/node_modules/git-off-my-land/scripts/post-install.js:44:6)
    at Module._compile (module.js:577:32)
    at Object.Module._extensions..js (module.js:586:10)
    at Module.load (module.js:494:32)
    at tryModuleLoad (module.js:453:12)
    at Function.Module._load (module.js:445:3)
    at Module.runMain (module.js:611:10)
    at run (bootstrap_node.js:387:7)
    at startup (bootstrap_node.js:153:9)
    at bootstrap_node.js:500:3
TypeError: fs.copyFileSync is not a function
    at Object.<anonymous> (/Users/littlm07/dev/id-account/node_modules/git-off-my-land/scripts/post-install.js:61:8)
    at Module._compile (module.js:577:32)
    at Object.Module._extensions..js (module.js:586:10)
    at Module.load (module.js:494:32)
    at tryModuleLoad (module.js:453:12)
    at Function.Module._load (module.js:445:3)
    at Module.runMain (module.js:611:10)
    at run (bootstrap_node.js:387:7)
    at startup (bootstrap_node.js:153:9)
    at bootstrap_node.js:500:3
- string-width@1.0.2 node_modules/nyc/node_modules/yargs/node_modules/cliui/node_modules/string-width
id-account@1.0.0 /Users/littlm07/dev/id-account
├── UNMET PEER DEPENDENCY eslint-plugin-unicorn@3.0.1
├── git-off-my-land@2.2.5
├─┬ id-http-client@4.1.2 invalid
│ └── bbc-http-client@9.3.0  extraneous (git+ssh://git@github.com/bbc/bbc-http-client-node.git#dc58840bcf471734cef9624b46a91af392f8c27d)
├── UNMET PEER DEPENDENCY istanbul@^0.4.3
├─┬ nyc@11.4.1
│ ├─┬ istanbul-reports@1.1.3
│ │ └─┬ handlebars@4.0.11
│ │   └─┬ uglify-js@2.8.29
│ │     └─┬ yargs@3.10.0
│ │       └─┬ cliui@2.1.0
│ │         └── wordwrap@0.0.2
│ └─┬ yargs@10.1.2
│   ├─┬ cliui@4.1.0
│   │ └─┬ strip-ansi@4.0.0
│   │   └── ansi-regex@3.0.0
│   └─┬ yargs-parser@8.1.0
│     └── camelcase@4.1.0
├── UNMET PEER DEPENDENCY react@16.2.0
└── UNMET PEER DEPENDENCY react-dom@16.2.0

npm WARN eslint-config-id@8.1.2 requires a peer of eslint-plugin-unicorn@^2.1.2 but none was installed.
npm WARN gulp-spawn-mocha@4.0.1 requires a peer of istanbul@^0.4.3 but none was installed.
npm WARN react-addons-test-utils@15.6.2 requires a peer of react-dom@^15.4.2 but none was installed.
npm WARN react-router@3.0.5 requires a peer of react@^0.14.0 || ^15.0.0 but none was installed.

@neilstuartcraig
Copy link
Owner

@MarcL - ah dang it! Thanks for the report, i'll see what i can do :-)

@neilstuartcraig
Copy link
Owner

Okey dokey, #8 should close this..if you could verify when you have a mo, that'd be ace @MarcL 😎

@MarcL MarcL closed this as completed Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants