refactor(index): use stringifyRequest to avoid absolute paths (loaderUtils.stringifyRequest)
#49
Conversation
|
|
|
@jimedelstein - You will need to sign the CLA before this can move forward |
|
@jimedelstein friendly ping |
michael-ciniawsky
changed the title
stringifyRequest to avoid absolute paths (loaderUtils.stringifyRequest)
|
@jimedelstein Please sign the CLA. Otherwise I'm going to close this PR within the next ~48-72 hours due to the missing CLA signment |
michael-ciniawsky
changed the title
stringifyRequest to avoid absolute paths (loaderUtils.stringifyRequest)stringifyRequest to avoid absolute paths (loaderUtils.stringifyRequest)
…ead of JSON.stringify
jimedelstein
force-pushed
the
remove-absolute-paths
branch
from
28a09cb
to
8010e42
Compare
|
@michael-ciniawsky I've signed it, unfortunately I didn't properly configure my git user when I made this commit. I was able to repair it and rebase it but I think I will have to make a new PR because it's still showing the default author info in addition to the correct info. Haven't had this problem before, please confirm that's the correct next step and there's not an easier way. Thanks |
|
(Squash the commits into one) and then using the |
|
@michael-ciniawsky That's what basically what I did (I used rebase instead of squash). When I wrote that it showed both committers, but now it seems to be working, so I guess we're good now? |
index.js
Outdated
| ")"; | ||
| }; | ||
| this.cacheable && this.cacheable(); | ||
| return "require(" + loaderUtils.stringifyRequest(this, "!!" + path.join(__dirname, "addScript.js")) + ")"+ |
There was a problem hiding this comment.
- "addScript.js")) + ")"+
+ "addScript.js")) + ")" +Missing Space
michael-ciniawsky
added
status: Pending
semver: Major
and removed
status: Blocked
semver: Patch
labels
|
@jimedelstein Please address the tiny nit mentioned #49 (comment) so I can proceed here :) |
|
@michael-ciniawsky done (finally!) |
|
@michael-ciniawsky any news on this? script-loader completely breaks consistent hashing across different machines if it's using absolute paths. See this article about it: https://medium.com/@the_teacher/webpack-different-assets-hashes-on-different-machines-the-problem-the-solution-ec6383983b99 Can this be looked at urgently? |
|
I too am very interested in this. Thanks. |
|
@michael-ciniawsky @evilebottnawi @sokra It's pretty rediclous that this hasn't been fixed yet. There's been a PR from the community trying to fix this issue since 2017, over 2 years and the last release of script-loader was back in Sep 2017 - is this project still maintained anymore? If the maintainers aren't willing to maintain it then can you at least deprecate/mark the project as abandoned because it really isn't fair to the community to let bugs like this sit around for so long. Appreciate all the work and time that goes into maintaining a project but we do need more feedback and guidance from the maintainers. |
Per webpack documentation on building loaders (https://webpack.github.io/docs/how-to-write-a-loader.html), loaders should not use absolute paths, but this one does. This PR fixes the problem.