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

lib: update to mkdirp@1 w/ Promises interface #2075

Closed
wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Mar 18, 2020

Ref #2074

I did want to update to 0.5.3 but Isaac has deprecated all 0.x versions. To avoid the deprecation warning we need to go to @1 which has a Promises interface. We don't have any async/await in 5.x so I don't want to introduce it. We do have one use of Promises already so this isn't a major change. I'd just like some checking of my code!

@rvagg
Copy link
Member Author

rvagg commented Mar 18, 2020

Node 6 tests fail, mkdirp now uses varargs: opts = { mode: 0o777 & (~process.umask()), fs, ...opts }

So ... how do we feel about dropping Node 6 support in this old release line? Is 6 ancient enough to stop caring or should we preserve 6 support for those people stuck on it? Alternatives I see if we want to maintain 6 support are (a) implement our own mkdirp for Node versions that don't have recursive, our needs are simpler than the use-cases mkdirp covers or (b) stick with 0.5.3 which will fix the security warning but include a deprecation warning.

@rvagg
Copy link
Member Author

rvagg commented Mar 19, 2020

Was reminded by #2047 that we have a request deprecation in place anyway, so an additional deprecation probably isn't a big deal.

My inclination here is to use mkdirp 0.5.3 to avoid the security warning but swallow the deprecation warning and not impact our Node version compatibility. We can move on with node-gyp 7 (6 is probably finished now) where we can start using fs.mkdir recursive where available and switch away from request (undecided how yet).

It really depends on what npm wants to do, if they jump to mkdirp 1 then we should probably follow suit to dedupe.

@darcyclarke I hear you're the one handling this next npm release? Do you want to coordinate on this or will you just be bumping to 0.5.3 (which our semver range allows).

@darcyclarke
Copy link
Member

@rvagg I'm going to push out a release to npm in the next little bit here which should pull in the latest mkdirp0.5.3 & resolve this.

Copy link

@O330oei O330oei left a comment

Choose a reason for hiding this comment

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

add

@rvagg rvagg closed this May 13, 2020
@rvagg rvagg deleted the rvagg/v5.x-deps-update branch May 13, 2020 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants