search

broccolijs / broccoli-persistent-filter

MIT License
12 stars 33 forks source link

Node 10 doesn't support recursive directory operations. #197

Closed chriseppstein closed 3 years ago

chriseppstein commented 4 years ago

So the 3.0 release has a bug in node 10 that evidently isn't covered by the test cases.

This project is using @types/node@^13 which didn't flag this as an issue. (I recommend we downgrade to ^10).

According to the node.js docs, recursive rmdir was added in 12.10.0.

The API is used here: https://github.com/broccolijs/broccoli-persistent-filter/blob/master/index.ts#L228-L229

Maybe the right fix is for the fs-merger to shim the node-12 api when in node 10?

cc: @SparshithNR @stefanpenner

rwjblue commented 4 years ago

Oiy. Good catch @chriseppstein. I think properly configuring eslint-plugin-node would help catch these kinds things too.

SparshithNR commented 4 years ago

@chriseppstein I noticed this when I added this functionality to this.output. So for now if we set recursive=true, I fall back to removeSync from fs-extra You can find it broccoli-output-wrapper.

https://github.com/SparshithNR/broccoli-output-wrapper/blob/27e307db9d5fb633d1676a43e4b87b1e0d416963/src/index.ts#L51-L53

mkdirSync has recursive support from node10 onwards.

chriseppstein commented 4 years ago

@SparshithNR Aha! That explains why there wasn't an error.

So the issue then is just with the types in broccoli-output-wrapper which declares that it's using the node.fs interface when it actually has slightly different support and generates an error when using @types/node@10. I'll file a bug against that repo.