feat(rimraf): add fs.rm migration recipe#486
Conversation
|
thanks for being volunteering but you have 4 opened PRs we didn't have a lot of maintainer on this initiative so plz don't open any other pr. |
AugustinMauroy
left a comment
AugustinMauroy
left a comment
There was a problem hiding this comment.
- give name to tests cases
- add remove dep step but before calling the function analyse the json ast to know if rimraf isn't used as cli
- add jsdocs to workflow
|
Thanks, understood. I won't open more PRs and will focus on the existing ones. I pushed the requested updates in the latest commit. |
| 'rimraf-v3', | ||
| 'rimraf-v4', | ||
| 'rimraf-v5', |
There was a problem hiding this comment.
I think nearly nobody use this specifier.
| rule: { kind: 'pair' }, | ||
| }); | ||
|
|
||
| return scriptPairs.some((pair) => { |
There was a problem hiding this comment.
here I think we can simplify by catching string_content kind node inside string kind and on string_content you can add regex that have rimraf so no need to have js regex ...
| /** | ||
| * Splits a comma-separated argument list without splitting nested expressions. | ||
| */ | ||
| const splitTopLevelArguments = (source: string): string[] => { |
There was a problem hiding this comment.
@brunocroh I pretty sure that you will have a 3 line function that can simplify that
| rimrafImports.forEach((importNode, index) => { | ||
| edits.push(importNode.replace(index === 0 ? importReplacement : '')); | ||
| }); |
There was a problem hiding this comment.
use for(const ... of ...) with an if else statement inside it's will be much more easiest to read
| rimrafImports.forEach((importNode, index) => { | ||
| edits.push(importNode.replace(index === 0 ? importReplacement : '')); | ||
| }); | ||
|
|
There was a problem hiding this comment.
| if (!edits.lenght) return null; |
for security
| - custom retry behavior | ||
| - custom glob options | ||
| - async glob deletes with callbacks | ||
| - package-specific fallback behavior on Windows |
There was a problem hiding this comment.
| - package-specific fallback behavior on Windows | |
| - package-specific fallback behavior on Windows | |
| - CLI usage isn't touch you may want to use something else |
AugustinMauroy
added
awaiting author
2 participants
Fixes #472.\n\n## Summary\n- add a rimraf-to-fs-rm recipe for literal rimraf deletes\n- handle v3 callback calls, v4/v5 promise calls, v5 sync calls, aliases, and sync glob expansion\n- document manual-review cases for runtime-specific rimraf behavior\n\n## Validation\n- npm.cmd run test --workspace @nodejs/rimraf-to-fs-rm\n- npm.cmd run type-check\n- npm.cmd run lint