terascope / teraslice

Scalable data processing pipelines in JavaScript
https://terascope.github.io/teraslice/
Apache License 2.0
50 stars 13 forks source link

isbinaryfile dependency update breaking yarn add teraslice-cli in assets CI workflow #3544

Closed busma13 closed 6 months ago

busma13 commented 7 months ago

isbinaryfile updated node engine to >20.0.0 in version 5.0.1. They then downgrade node version to >18.0.0 in version 5.0.2. Version 5.0.0 is the last version that supports node 16.

godber commented 7 months ago

It's worth pointing out that we're just about ready to cut away from Node 16, we have on internal project that hasn't shipped all the way to prod on the Node 18 build, but does work and has been working in test environments. Hypothetically we could drop 16 support and move on to adding 20 and this problem would go away. The risk being that we find some reason we can't get that internal project shipped on 18 and need some test coverage on Node 16 still.

godber commented 7 months ago

The upstream package rolled back to node 16 here: https://github.com/gjtorikian/isBinaryFile/issues/70

If he rolled back to 16 our problem would go away, but I almost feel bad asking it.

We've tried about 4 different ways of trying to constrain this dependency to and older version without any luck.

godber commented 7 months ago

The only additional progress I've made on this is that it's pretty clear that yarn only supports resolutions at the top level package.json in the case of workspaces/monorepos.

busma13 commented 7 months ago

Pulling teraslice-cli files out of the monorepo and into their own project, then running yarn install results in the correct versions of isbinaryfile. I can also run yarn global add file:<path to standalone teraslice-cli> and I get the same yarn.lock file with the correct versions. It is only when the package is pulled directly from npm that the yarn.lock is built with an isbinaryfile with version 5.0.2.

godber commented 7 months ago

Deleting yarn.lock on master and re-running yarn results in a similar engine error but for Elasticsearch:

error elasticsearch8@8.12.1: The engine "node" is incompatible with this module. Expected version ">=18". Got "16.20.2"
verbose 27.735799503 Error: Found incompatible module.
    at checkOne (/Users/godber/Workspace/terascope/opensource/teraslice/.yarn/releases/yarn-1.22.19.js:48581:11)
    at Object.check (/Users/godber/Workspace/terascope/opensource/teraslice/.yarn/releases/yarn-1.22.19.js:48600:5)
    at /Users/godber/Workspace/terascope/opensource/teraslice/.yarn/releases/yarn-1.22.19.js:6894:73
    at Generator.next (<anonymous>)
    at step (/Users/godber/Workspace/terascope/opensource/teraslice/.yarn/releases/yarn-1.22.19.js:310:30)
    at /Users/godber/Workspace/terascope/opensource/teraslice/.yarn/releases/yarn-1.22.19.js:321:13
error Found incompatible module.
godber commented 7 months ago

At this point, I think I'll advocate for eliminating Node 16 from our support matrix and hope we're not forced to do any more releases on it. I will spend a few minutes seeing what all uses the yeoman dependencies and if they actually can be tossed out. Though I have a suspicion that it's used for more than just templating asset generation.

godber commented 7 months ago

I don't think we can just eliminate the yeoman-generator as a dependency. It is used by generator-teraslice which generates the asset registries in addition to generating sample assets. So it's not something we can trivially eliminate.

busma13 commented 6 months ago

The reason our resolutions from the teraslice-cli package.json are not being used when we run yarn global add teraslice-cli is because that command is adding a dependency to the yarn global 'project' package.json. Because resolutions are only followed if in the root package.json of a project, this is where we must add resolutions.

For example:

package.json:

{
  "dependencies": {
    "jest": "^29.7.0",
    "node-gyp": "^9.4.0",
    "teraslice-cli": "^0.61.5",
    "typescript": "^5.3.3"
  }
}

If I add a resolutions section with the packages/versions I want to limit it will look like this:

{
  "dependencies": {
    "jest": "^29.7.0",
    "node-gyp": "^9.4.0",
    "teraslice-cli": "^0.61.5",
    "typescript": "^5.3.3"
  },
  "resolutions": {
    "isbinaryfile": "<5.0.1",
    "yeoman-environment/isbinaryfile": "4.0.10"
  }
}

Now running yarn global add teraslice-cli while on node 16 will install teraslice-cli without a node engine error.

This modification of the package.json could be done within a CI runner if we run into this type of issue in the future.

busma13 commented 6 months ago

We updated the node version to bypass this issue. Closing