Update shelljs to avoid circular dependency warnings on Node 14#184
Update shelljs to avoid circular dependency warnings on Node 14#184nfischer merged 1 commit intoshelljs:masterfrom Pomax:patch-1
Conversation
shelljs/shelljs#973 updated shelljs to prevent Node 14 from flooding the user with hundreds of lines that look like: ``` (node:117) Warning: Accessing non-existent property 'cd' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'chmod' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'cp' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'dirs' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'pushd' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'popd' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'echo' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'tempdir' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'pwd' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'exec' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'ls' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'find' of module exports inside circular dependency (node:117) Warning: Accessing non-existent property 'grep' of module exports inside circular dependency ```
|
This shouldn't be necessary. Semver behavior dictates npm should pick the highest available patch version matching |
|
That's not what you want in this case though: 0.8.4 is effectively a compatibility fix, so you want to peg the minimum version to 0.8.4 and nothing prior to that. |
|
If anyone downloads shx, they'll already receive shelljs@0.8.4. This is thanks to the |
|
Not if they rely on cached registries they won't? (e.g. CI/CD systems) This change makes sure that registry caching sees that it really has to be 0.8.4 or up, and not 0.8.1 or up. Let's keep people's logs readable =( |
Codecov Report
@@ Coverage Diff @@
## master #184 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 114 114
=========================================
Hits 114 114 Continue to review full report at Codecov.
|
|
Thank you for explaining your concern for cached registries. My understanding is the cached registry must be updated to choose v0.8.4 as the preferred ShellJS version. I assume cached registries should already have a process for taking patch releases which include bug/compatibility/security fixes. As a friendly reminder for future contributions, please avoid hyperbolic language to ensure we maintain a respectful discussion. Ex. "spew out 100s of warnings" might have been better phrased as "emit circular dependency warnings." This is maintained in my spare time, so I appreciate respectful language for bug reports and patches. |
|
Fair enough, although our build log literally had 100s of warnings, that wasn't hyperbole (but pasting 400 line logs is generally not super useful when filing an issue). |
shelljs/shelljs#973 updated shelljs to prevent Node 14 from flooding the user with hundreds of lines that look like:
And it would be a good idea to make sure that the minimum version of shelljs matches that updated version.