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

Npm install can break server-side path to client-side dependency /assets/superagent.js #185

Closed
joelpurra opened this issue Apr 8, 2020 · 4 comments · Fixed by #187
Closed

Comments

@joelpurra
Copy link

The client-side dependency /assets/superagent.js is given special server-side path treatment in index.js.

On my system /assets/superagent.js returns HTTP error 404 and breaks slackin-extended client-side. This is due to (changes in) how npm install places dependencies, which may be optimized between packages in the same package.json.

Versions

Am running slightly older versions due to apt on Debian 10 Buster.

node --version
v10.15.2
npm --version
6.14.4
Paths
npm ls superagent
[email protected] .../slackin.joelpurra.com/public_html
└─┬ [email protected]
  └── [email protected] 
ls --format=long --directory ./node_modules/{slackin-extended,superagent}
drwxr-xr-x 1 root root 116 Apr  8 16:56 ./node_modules/slackin-extended
drwxr-xr-x 1 root root 282 Apr  8 16:56 ./node_modules/superagent

Perhaps something like resolve-pkg could be used in the short-term, but replacing superagent with something else might be a better long-term solution. See also #86.

Originally posted by @joelpurra in #86 (comment)

@XhmikosR
Copy link
Collaborator

XhmikosR commented Apr 8, 2020

@joelpurra: Can you try this branch please? https://github.com/XhmikosR/slackin-extended/tree/fix-superagent

@joelpurra
Copy link
Author

joelpurra commented Apr 8, 2020

@XhmikosR: due to the locked-down nature of the particular system where the issue was discovered, I can't test your branch in the exact same way as before.

Tested (without the production slack configuration) on a dev machine (with the same/similar node_modules structure; slightly abbreviated below) though, and the resolved superagentDist in the log output points to the correct file on disk. Should work!

Versions
node --version
v12.16.1
npm --version
6.13.4
Paths
npm ls superagent
[email protected] .../slackin-extended-test
└─┬ [email protected] (github:XhmikosR/slackin-extended#72003bf158cc7cfd2f9997451815fc079948234c)
  └── [email protected]
ls -l -d ./node_modules/{slackin-extended,superagent}
drwxr-xr-x  10 joelpurra  staff  320 Apr  8 20:35 ./node_modules/slackin-extended
drwxr-xr-x  17 joelpurra  staff  544 Apr  8 20:35 ./node_modules/superagent
>> superagentDist: .../slackin-extended-test/node_modules/superagent/superagent.js

@XhmikosR
Copy link
Collaborator

XhmikosR commented Apr 8, 2020

Cool, it was easier to fix than I thought then 🙂

PR: #187

@emedvedev
Copy link
Owner

Thank you :) Let me test the PR and merge.

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

Successfully merging a pull request may close this issue.

3 participants