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

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

Closed
chriseppstein opened this issue May 27, 2020 · 3 comments · Fixed by #198
Closed

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

chriseppstein opened this issue May 27, 2020 · 3 comments · Fixed by #198

Comments

@chriseppstein
Copy link
Contributor

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
Copy link
Member

rwjblue commented May 28, 2020

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

@SparshithNR
Copy link
Contributor

@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
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants