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

Loosen engines restriction, which currently requires Node >= 12.17.0 #11

Closed
bcoe opened this issue Oct 12, 2020 · 4 comments
Closed

Loosen engines restriction, which currently requires Node >= 12.17.0 #11

bcoe opened this issue Oct 12, 2020 · 4 comments

Comments

@bcoe
Copy link

bcoe commented Oct 12, 2020

A recent release of @compodoc/ngd-transformer pulls in @aduh95/viz.js, which drops support of Node 10.

This is breaking upstream builds of google-auth-library.

Are there any load-bearing Node 12 features being used, or could you consider loosening the engines constraint to include Node 10?

Refs: compodoc/ngd#68

@aduh95
Copy link
Owner

aduh95 commented Oct 12, 2020

Are there any load-bearing Node 12 features being used

Well, there are ESM, package exports, and worker_threads which are not available by default on Node.js 10.x

Worth noting the library also exports a @aduh95/viz.js/sync script which doesn't use any of those features, and may even be compatible with Node.js v6.x+. compodoc is using that very script (https://github.com/compodoc/ngd/blob/60dbb2ac17e1d87f75b2378975607e578ae2a1fd/src/modules/transformer/dist/engines/dot/dot.js#L193), so that means the test failures on the google-auth-library PR are actually false positive.

I'm fine with loosening as I understand there are folks out there who need support for Node.js v10.x, but I'm not sure what's the best way to signal to users that parts of the API will break on older versions.

@bcoe
Copy link
Author

bcoe commented Oct 14, 2020

I'm fine with loosening as I understand there are folks out there who need support for Node.js v10.x, but I'm not sure what's the best way to signal to users that parts of the API will break on older versions.

Perhaps you could add Node 10 to your test matrix, but only run the sync tests? This would ensurer that they continue working until Node 10 is EOL (soonish). I think it's okay to then expect folks upstream to use the appropriate part of the API -- we're only talking about a few months I believe?

I ideally don't want to turn off --engines-strict, because as we move towards the Node 10 EOL catching dependencies that require Node 12 is very important.

@bcoe
Copy link
Author

bcoe commented Oct 16, 2020

@aduh95 I worked around this upstream by moving our --engine-strict test to npm i --production only, so this is no longer a high priority for me.

@aduh95 aduh95 closed this as completed in 56c780a Oct 17, 2020
@aduh95
Copy link
Owner

aduh95 commented Oct 17, 2020

I've removed the engine restriction and publish a new version. My plan is to re-introduce it after Node.js v10 reaches end-of-life.

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

No branches or pull requests

2 participants