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

[Bug]: depRange.replaceAll is not a function #32

Closed
1 task done
ThisIsMissEm opened this issue Feb 9, 2023 · 3 comments · Fixed by #37
Closed
1 task done

[Bug]: depRange.replaceAll is not a function #32

ThisIsMissEm opened this issue Feb 9, 2023 · 3 comments · Fixed by #37
Labels
bug Something isn't working

Comments

@ThisIsMissEm
Copy link

Expected Behavior

I was expecting a failing report (this is a node.js v14.20.0 project with some really bad/messy dependencies)

Actual Behavior

The following output:

$ npx depngn 18
Error: TypeError: depRange.replaceAll is not a function
    at /Users/redacted/.npm/_npx/80daaf8cfdca6619/node_modules/depngn/dist/cli.cjs:7609:27
    at step (/Users/redacted/.npm/_npx/80daaf8cfdca6619/node_modules/depngn/dist/cli.cjs:69:23)
    at Object.throw (/Users/redacted/.npm/_npx/80daaf8cfdca6619/node_modules/depngn/dist/cli.cjs:50:53)
    at rejected (/Users/redacted/.npm/_npx/80daaf8cfdca6619/node_modules/depngn/dist/cli.cjs:41:65)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

Looks like maybe one of the dependencies may be doing something weird with the engines field or something?

Steps to Reproduce

  1. Use node.js 14.x (yes, I know, it's well and truly out of date)
  2. Run depngn: npx depngn 18 on a project
  3. The actual behaviour is observed

If you want a repository you can try this in, then see: https://github.com/inrupt/pod-browser

Additional Information

I tried to add debug logging around the depRange line, and the only output it gave was { depRange: '>=6.9.0' } before failing. I tried doing a typeof on depRange and it said "string", so I did a typeof depRange.replaceAll and it said undefined.

It looks like in node.js 14.x, String.prototype.replaceAll does not yet exist (it's a newer language feature):

Welcome to Node.js v14.20.0.
Type ".help" for more information.
> String.prototype.replaceAll
undefined
>

So if you want to support Node.js 14.x then you'll need to add a polyfill in.

What package manager are you seeing the problem with?

npm

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ThisIsMissEm ThisIsMissEm added the bug Something isn't working label Feb 9, 2023
@ThisIsMissEm
Copy link
Author

I added in the polyfill from https://vanillajstoolkit.com/polyfills/stringreplaceall/ to .npm/_npx/80daaf8cfdca6619/node_modules/depngn/dist/cli.cjs and it now works in node.js 14.x, but would still require a patch from your side.

Looks like you're intending to support Node.js >= 10.0.0:

  "engines": {
    "node": ">=10.0.0"
  },

@arielj
Copy link
Contributor

arielj commented Feb 9, 2023

Maybe we can use a global regexp to not need a polyfill? like replace(/something/g, ...), that g flag should work like a replaceAll

@ThisIsMissEm
Copy link
Author

That's basically what the polyfill does anyway!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants