Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

upgrade dependency to address graceful-fs issue #15

Merged
merged 7 commits into from
Jul 19, 2018

Conversation

shiftkey
Copy link
Contributor

I stumbled upon an obscure issue with this snippet of code under NodeJS 10.7.0 and multiple versions of ts-node (5.x and 7.x):

console.log('get to here')

import * as legalEagle from 'legal-eagle'

console.log(`but we don't get to here`)

Which at runtime would do this:

get to here
 [6804]: src\node_contextify.cc:635: Assertion `args[1]->IsString()' failed.
 1: 00007FF764420205
 2: 00007FF7643FA176
 3: 00007FF7643FA241
 4: 00007FF7643D3F9A
 5: 00007FF764A34102
 6: 00007FF764A35258
 7: 00007FF764A345BD
 8: 00007FF764A344DB
 9: 0000012BE9B841C1
error Command failed with exit code 134.

Searching for that message pointed to a few threads about graceful-fs and it's use of natives breaking on future versions of Node: nodejs/node#19786 (comment)

And sure enough, I poke at what yarn list shows this is one of the legal-eagle dependencies:

...
├─ legal-eagle@0.15.0
│  ├─ read-installed@3.1.3
│  └─ underscore@~1.6.0
...
├─ read-installed@3.1.3
│  ├─ debuglog@^1.0.1
│  ├─ graceful-fs@2 || 3
│  ├─ graceful-fs@3.0.11
│  │  └─ natives@^1.1.0
│  ├─ read-package-json@1
│  ├─ readdir-scoped-modules@^1.0.0
│  ├─ semver@2 || 3 || 4
│  ├─ semver@4.3.6
│  ├─ slide@~1.1.3
│  └─ util-extend@^1.0.1
...

You can work around this in the interim by adding a resolutions entry to your package.json to force it to use a later version of graceful-fs:

  "resolutions": {
    "graceful-fs": "4.1.11"
  }

But this will likely add noise to your logs:

$ yarn
yarn install v1.7.0
[1/5] Validating package.json...
[2/5] Resolving packages...
⠁ (node:4644) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
warning Resolution field "graceful-fs@4.1.11" is incompatible with requested version "graceful-fs@2 || 3"
warning Resolution field "graceful-fs@4.1.11" is incompatible with requested version "graceful-fs@2 || 3"
...

The better fix is to upgrade read-installed to a version that uses graceful-fs 4.x, which this PR does. I wasn't able to reproduce this in isolation, but I'm reasonably happy that the investigations above mean this is a Good Thing To Do™.

There weren't any tests to verify against this, but I've tested this version in Desktop (where the problem originated) and confirmed the behaviour is unaffected.

cc @iAmWillShepherd @nerdneha in case you stumbled upon this earlier

@shiftkey shiftkey requested a review from lee-dohm July 19, 2018 14:33
@lee-dohm
Copy link
Contributor

@daviwil This falls under the Node 10.x stuff that you've been keeping an eye on?

@daviwil
Copy link
Contributor

daviwil commented Jul 19, 2018

Added Node 8 and 10 to the build matrix to make sure our currently-used versions work with the new dependencies but I caused a new CI issue, will get that fixed.

@daviwil
Copy link
Contributor

daviwil commented Jul 19, 2018

Everything's looking good! Merging this, thanks a lot @shiftkey!

@daviwil daviwil merged commit ace984c into master Jul 19, 2018
@daviwil daviwil deleted the upgrade-dependencies branch July 19, 2018 23:31
@daviwil
Copy link
Contributor

daviwil commented Jul 19, 2018

Just published 0.16.0

@shiftkey
Copy link
Contributor Author

@daviwil awesome, thanks!

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

Successfully merging this pull request may close these issues.

3 participants