search

Financial-Times / nori

🍙 exploratory command-line tool to make changes across multiple repositories & track their progress
MIT License
11 stars 0 forks source link

Pin enquirer dependency to v2.3.0 #124

Closed andygout closed 4 years ago

andygout commented 4 years ago

Fixes https://github.com/Financial-Times/nori/issues/106.

Globally installed

$ npm i -g nori $ npm list: enquirer@2.3.6 $ nori $ ? create a session › foobar Results in:

(node:1537) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'filter' of undefined
    at MultiSelect.filter (/Users/samuel.parkinson/Repositories/Financial-Times/nori/node_modules/enquirer/lib/types/array.js:509:26)

My guess would be that the same thing is happening when running Nori via npx nori.

Installing and running repo locally in its current state

$ git clone git@github.com:Financial-Times/nori.git $ npm i $ npm list: enquirer@2.3.0 (the package.json lists ^2.3.0 but the package.lock lists 2.3.0) $ ? create a session › foobar Results in: ? available operations … (i.e. no error)

Installing and running repo locally with v2.3.6 of enquirer (via pinning to that version in package.json)

$ git clone git@github.com:Financial-Times/nori.git $ npm i $ npm list: enquirer@2.3.6 $ ? create a session › foobar Results in:

(node:1537) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'filter' of undefined
    at MultiSelect.filter (/Users/samuel.parkinson/Repositories/Financial-Times/nori/node_modules/enquirer/lib/types/array.js:509:26)

This is the quick fix. I am curious enough to see exactly which 2.3.x version of enquirer was the one where this error first appeared, and see if the changes made for that version can be accommodated by the nori code so that we can keep consuming updates.

What this scenario has indicated is that the package.lock is ignored when acquiring this tool via npx nori or npm i -g nori, which is interesting and perhaps unexpected.

andygout commented 4 years ago

v2.3.0 was the last version of enquirer that does not throw the Cannot read property 'filter' of undefined error.

As of v2.3.1 the error is thrown. I will take a look at whether there is anything obvious that might be causing this.

apaleslimghost commented 4 years ago

What this scenario has indicated is that the package.lock is ignored when acquiring this tool via npx nori or npm i -g nori, which is interesting and perhaps unexpected.

that's expected, package-lock.json is for local installs (i.e. npm install in the repo) only. the equivalent for people consuming the package is npm-shrinkwrap.json, which historically has been problematic but should be fine to use since npm 5

andygout commented 4 years ago

Comparison between v2.3.0 and v2.3.1 (57 commits and 35 files changes and no release notes for v2.3.1…): https://github.com/enquirer/enquirer/compare/2.3.0...2.3.1

andygout commented 4 years ago

v2.3.1 included this change in lib/types/array.js:

- let result = this.choices.filter(fn);
+ let choices = this.options.multiple ? this.state._choices : this.choices;
+ let result = choices.filter(fn);

Indicating that this.options.multiple evaluates to true but that this.state._choices is undefined.

I am included to go with the fix proposed by this PR rather than dig in any further.