Open happybeing opened 5 years ago
happybeing
commented
5 years ago Update, this problem is NOT present in: 7a5d2637f4421bf293d3f65d370cb9bb2bd6a1ba "Drop babel-minify-webpack-plugin entirely" by TimBL
and has been introduced at: a0dddfe28d2527ff8f99116b798aeb362ca01fd9 "Update fetcher.js to conditionally require solid-auth-cli in browserless mode" by @jeff-zucker
I shall poke further.
jeff-zucker
commented
5 years ago Looks like I didn't include fs as a dependency in solid-auth-cli. I've added it and updated the npm to v0.1.13. Sorry for my npm cluelessness. Please let me know if that fixes the problem.
happybeing
commented
5 years ago Update: hold fire, looks like it may be a problem in my lib!
happybeing
commented
5 years ago Oh, wait, maybe I just picked that up!
happybeing
commented
5 years ago Thanks @jeff-zucker - solid-auth-cli 0.1.13 hasn't fixed it but I get a different error, one which I saw when I added 'fs' directly to rdflib.js/node_modules/node-auth-cli. The error is now:
ModuleNotFoundError: Module not found: Error: Can't resolve 'fs' in '/home/mrh/src/solid/rdflib.js/node_modules/file-fetch'
...But hang on a bit Jeff as while trying to create 'how to reproduce the issue' instructions it seemed to disappear (that's why I thought maybe I'd picked up your change). Might just be finger trouble. :grimacing:
Plenty of npm cluelessness to go around, so feel free to hand on while I try to replicate it from scratch.
happybeing
commented
5 years ago @jeff-zucker update... your fix changes the error but I'm still getting an error related to 'fs'. I have a workaround, and include that plus details of how to replicate the problem below:
Workaround (mirroring 96902f5d70c158ab8d201fdbdede4a2241d44fc1) is for me to add the following to my safenetworkjs/webpack.config.js:
externals: {
'solid-auth-cli': 'null',
},The above appears to work, but the problem seems to be with rdflib.js, as something is still pulling in safe-auth-cli and causing the issue with 'fs', but only when it is used with npm link!
To replicate the issue in the OP (note you need yarn as well as npm):
git clone https://github.com/linkeddata/rdflib.js
cd rdflib.js
npm install
npm link
cd ..
git clone https://github.com/theWebalyst/safenetworkjs
cd safenetworkjs
yarn
git checkout development
yarn install
npm link rdflib
yarn buildsac
# Above results in error:
# ModuleNotFoundError: Module not found: Error: Can't resolve 'fs' in '/home/mrh/ttt/rdflib.js/node_modules/solid-auth-cli/src'
# So upgrade solid-auth-cli to v0.1.13 and retry:
cd ../rdflib.js
npm upgrade solid-auth-cli
cd ../safenetworkjs
yarn buildsac
# Now get different error:
# ModuleNotFoundError: Module not found: Error: Can't resolve 'fs' in '/home/mrh/ttt/rdflib.js/node_modules/file-fetch'If I then apply the workaround, the above error disappears (it also solved the similar error with the earlier solid-auth-cli)
jeff-zucker
commented
5 years ago It looks like file-fetch (which solid-auth-cli requires to support the file:// scheme) requires "fs" without listing it as a dependency, I'll file an issue. In the longer run, @RubenVerborgh and I are working on an isomorphic auth which should obviate the need for setting solid-auth-cli to null in webpack and the conditional require in rdflib.js. Ruben's idea is that libraries which can work in both nodejs and browser contexts (like rdflib and query-ldflex) will require solid-auth-cli which will fallback to solid-auth-client for the browser build. See https://github.com/RubenVerborgh/LDflex-Comunica/issues/12#issuecomment-475296533
RubenVerborgh
commented
5 years ago It looks like file-fetch (which solid-auth-cli requires to support the file:// scheme) requires "fs" without listing it as a dependency
fs is built-in in Node?
jeff-zucker
commented
5 years ago Hmm, yes looks like it is built in. I wonder why npm is having trouble resolving it.
bergos
commented
5 years ago Just checked if file-fetch does any fancy stuff when it requires fs, but it's a plain require.
Is this an error from Webpack? Then I think there should be already a dummy fallback for file-fetch and the package should not be used at all in the browser. If proto-fetch is used, don't register the file protocol in the browser.
If it's in node, is it some kind of sandboxing?
jeff-zucker
commented
5 years ago @theWebalyst clarified in chat that the issue only occurs when accessing rdflib via npm link, not with rdflib by itself. @konobi pointed out that npm link brings some complications related to module resolution and to webpack, see https://medium.com/@penx/managing-dependencies-in-a-node-package-so-that-they-are-compatible-with-npm-link-61befa5aaca7.
I previously worked with a local fork of
rdfliband was successfully usingnpm link/npm link rdflibto allow me to develop other code that depended on the fork.Since updating to
rdflibv0.20.0 my project builds successfully, but fails if I try to build it with a local fork vianpm link. I get the same problem using an unadulterated clone of the latestrdflib(v0.20.0). I also tried using yarn but get the same errors (see below).The library using
rdflibhas been switched fromnpmtoyarnso I'm looking into that, but don't think it is likely to be the cause. So posting this to see if anyone is currenly usingnpm linkto developrdflib.