search

bholloway / resolve-url-loader

Webpack loader that resolves relative paths in url() statements based on the original source file
563 stars 70 forks source link

limit file search by `attempts` option #70

Closed bholloway closed 6 years ago

bholloway commented 6 years ago

Addresses issue #69.

bholloway commented 6 years ago

@mvastola This is my current thinking.

I have done some basic tests here against the old-school bootstrap problem. It only tries the immediate directory (from the source-map) where the file is expected to be, and does not search any further.

resolve-url-loader: ../fonts/bootstrap/glyphicons-halflings-regular.eot
  /<redacted path>/bower_components/bootstrap-sass-official/assets/stylesheets/bootstrap
  NOT FOUND (INTERRUPTED)

That is on MacOs. I have not done any tests on Windows yet, but I do have a VM which I use for that purpose.

Out of interest, what platform are you on?

mvastola commented 6 years ago

I'm on Linux. Should I take this to PR to mean I should try this code change and see if it works?

bholloway commented 6 years ago

@mvastola It should work now. I would say the answer is yes, but if you experience problems then don't waste any time on it.

bholloway commented 6 years ago

Also it is good you are on Linux, because it is the one OS that I don't actively test on. And there are subtle differences to MacOS sometimes.

mvastola commented 6 years ago

Ok. The good news is the security problem mentioned no longer presents itself. The bad news though is assets that should be found that are relative to the file with the url() directive seem to instead be searched for from the root path.

Here's the test repo I'm using (you can check it out if you want to run the compile yourself).

Also, question: this is the modification I made to webpacker to set attempts. Did I do that wrong? (I ask because I set debug too and I don't see debug output.)

Anyway, this is the error output I encountered when I went to compile the stylesheet with entry point ./app/javascript/application.scss from https://github.com/mvastola/webpacker-issue-932.

Version: webpack 3.7.1
Time: 2762ms
                              Asset      Size  Chunks             Chunk Names
application-afdfd4499c23d808acc3.js    8.4 kB       0  [emitted]  application
                      manifest.json  68 bytes          [emitted]  
   [0] ./app/javascript/packs/application.js 29 bytes {0} [built]
   [1] ./app/javascript/application.scss 4.26 kB {0} [built] [failed] [1 error]
   [2] ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss 271 bytes [built] [1 warning]
    + 3 hidden modules

WARNING in ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss
(Emitted value instead of an instance of Error) postcss-smart-import: home/mvastola/src/bugs/webpacker-issue-932/app/javascript/css/foo.css:1:0: Failed to find '../scss/bar.scss'
    in [ 
        home/mvastola/src/bugs/webpacker-issue-932/app/javascript
    ]
 @ ./app/javascript/application.scss 4:14-263
 @ ./app/javascript/packs/application.js

ERROR in ./app/javascript/application.scss
Module build failed: ModuleNotFoundError: Module not found: Error: Can't resolve '../scss/bar.scss' in 'home/mvastola/src/bugs/webpacker-issue-932/app/javascript'
    at factoryCallback (home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/Compilation.js:274:40)
    at factory (home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:235:20)
    at resolver (home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:60:20)
    at asyncLib.parallel.e (home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:127:20)
    at home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:3861:9
    at home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:421:16
    at iteratorCallback (home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:996:13)
    at home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:906:16
    at home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:3858:13
    at resolvers.normal.resolve (home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:119:22)
    at onError (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:65:10)
    at loggingCallbackWrapper (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19)
    at runAfter (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:158:4)
    at innerCallback (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:146:3)
    at loggingCallbackWrapper (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19)
    at next (home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:252:11)
    at home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/UnsafeCachePlugin.js:40:4
    at loggingCallbackWrapper (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19)
    at runAfter (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:158:4)
    at innerCallback (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:146:3)
    at loggingCallbackWrapper (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19)
    at next (home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:252:11)
    at innerCallback (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:144:11)
    at loggingCallbackWrapper (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19)
    at next (home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:249:35)
    at resolver.doResolve.createInnerCallback (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/DescriptionFilePlugin.js:44:6)
    at loggingCallbackWrapper (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19)
    at afterInnerCallback (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:168:10)
    at loggingCallbackWrapper (home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19)
    at next (home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:252:11)

ERROR in ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss
Module not found: Error: Can't resolve '../scss/bar.scss' in 'home/mvastola/src/bugs/webpacker-issue-932/app/javascript'
 @ ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss 3:10-99
 @ ./app/javascript/application.scss
 @ ./app/javascript/packs/application.js
Child extract-text-webpack-plugin node_modules/extract-text-webpack-plugin/dist node_modules/css-loader/index.js??ref--5-2!node_modules/postcss-loader/lib/index.js??ref--5-3!node_modules/resolve-url-loader/index.js??ref--5-4!node_modules/sass-loader/lib/loader.js??ref--5-5!app/javascript/application.scss:
       [0] ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1,"absolute":false,"sourceMap":false,"engine":"rework","fail":false,"silent":false,"keepQuery":false,"root":null,"includeRoot":false}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss 271 bytes {0} [built] [1 warning]
        + 1 hidden module

    WARNING in ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1,"absolute":false,"sourceMap":false,"engine":"rework","fail":false,"silent":false,"keepQuery":false,"root":null,"includeRoot":false}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss
    (Emitted value instead of an instance of Error) postcss-smart-import: home/mvastola/src/bugs/webpacker-issue-932/app/javascript/css/foo.css:1:0: Failed to find '../scss/bar.scss'
        in [
            home/mvastola/src/bugs/webpacker-issue-932/app/javascript
        ]

    ERROR in ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1,"absolute":false,"sourceMap":false,"engine":"rework","fail":false,"silent":false,"keepQuery":false,"root":null,"includeRoot":false}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss
    Module not found: Error: Can't resolve '../scss/bar.scss' in 'home/mvastola/src/bugs/webpacker-issue-932/app/javascript'
     @ ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1,"absolute":false,"sourceMap":false,"engine":"rework","fail":false,"silent":false,"keepQuery":false,"root":null,"includeRoot":false}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss 3:10-99
bholloway commented 6 years ago

You would definitely expect to see some stdout when you have debug active.

Unfortunately I am a Webpack 1 dinosaur and can't definitely say your Webpack 2 config is correct. However it certainly looks (from the Error messages) that some debug:true is on the resolve-url-loader.

Where is the configuration actually located in your project? I'm thinking I need to get a working copy and yarn install to find that.

bholloway commented 6 years ago

@mvastola from your log it seems you are having some sort of problem importing scss. Is the url() statement in question actually processed by SASS?

I would suggest removing resolve-url-loader from your test project and checking the vanilla SASS feature set is working as expected. Even if that means removing all the url() statements to get it passing.

Failing that I shall fork and take a deeper look at your example.

mvastola commented 6 years ago

Yes. The issue is definitely right here. The problem is that this path is being evaluated relative to folder of the entry file rather than the file containing the url directive (./app/javascript/css). The file that should be found by url(../scss/bar.scss) is ./app/javascript/scss/bar.scss.

I tried disabling resolve-url-loader completely (by commenting out this line). The error is identical with the exception that the error output is no longer reflecting that resolve-url-loader is in the chain of loaders. This is the expected behavior.

If I alter the url() in ./app/javascript/css/foo.css to instead be a path relative to ./app/javascript (so just url(./scss/bar.scss)) everything compiles (with or without resolve-url-loader) but I don't get the expected result with resolve-url-loader on. When resolve-url-loader is on, the url resolved in ./app/javascript/scss/bar.scss should point to ./app/javascript/scss/images/blue.png. Instead, it points to ./app/javascript/images/blue.png, again relative to the entry file.

mvastola commented 6 years ago

@mvastola from your log it seems you are having some sort of problem importing scss. Is the url() statement in question actually processed by SASS?

No, the url() statement is in a css file. It shouldn't be processed by sass. The URL should be resolved by your loader though.

Unfortunately I am a Webpack 1 dinosaur and can't definitely say your Webpack 2 config is correct. However it certainly looks (from the Error messages) that some debug:true is on the resolve-url-loader. Where is the configuration actually located in your project? I'm thinking I need to get a working copy and yarn install to find that.

Unfortunately the webpack config is mostly generated on the fly by rails magic, so I can't really output it and just show you.

I will however give you directions on how clone my repo to debug. Just give me a few.

Does this (from the error backtrace) look like it should correctly pass debug to the loader? If so, then I set it right..

./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss
mvastola commented 6 years ago

Here are instructions on duplicating. Not sure how fluent you are in ruby/rails so apologies if they are a bit too basic for you. (On the other hand, if you need more help, I'm more than happy to give it.)

Prerequisites

Setup

To Compile Stylesheets/Images

Assets will be generated into ./public/packs if successful. If everything works correctly, ./public/packs/images/blue-[hash].png should contain a blue (not red or yellow) square.

(A red blue.png means the image was fetched relative to the entry file even though there was a suitable match relative to the bar.scss file. A yellow blue.png means something even weirder happened.)

bholloway commented 6 years ago

Yes. The issue is definitely right here.

@mvastola I am not sure whether resolve-url-loader will pick up that url() in the @import.

Your entry is .scss then I would expect sass-loader to handle this .css file and its @import. I would expect that whole line to be removed by the preceding sass-loader and that the resolve-url-loader would not even see it.

However I suspect it is being ignored at sass-loader because it is wrapped in url() and therefore remains in the output to cause errors for postcss-loader. Which is why you are relying on the resolve-url-loader to correct the path. Is all of this correct?

mvastola commented 6 years ago

Ok. You are giving me a bit too much credit here. I don't understand too much about what sass-loader, css-loader and postcss-loader each do (though I understand that one of them generates the source-map for resolve-url-loader to use/modify.

I just constructed this use case because I thought it was complicated enough that it would test everything that we needed it to in one fell swoop and if it worked we could safely assume everything was working. I didn't mean for it to take advantage of a feature that doesn't exist, if that's what I made it do.

I'm a bit confused though.. in what order are these loaders run? I wrapped the @import in a url() because (AFAIK) in a css file I can't leave out the url() (can I?).

The loader string is the following

./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss

If you're talking of sass-loader ignoring the directive, that implies sass-loader is the first loader used (otherwise it's the last and wouldn't be relevant since the error is in postcss-loader). If sass-loader is the first loader, wouldn't resolve-url-loader be the second and thus have an opportunity to rewrite the url in question? Or am I missing something?

Apologies if I'm sending us on a wild goose case.

As I said though, even when I corrected the URL here to be url(./scss/bar.scss), the code compiled, but it matched the wrong blue.png. So I'm suspicious that these issues might be interrelated and resolve-url-loader may simply not be invoking. (I also don't recall seeing any debug output.)

bholloway commented 6 years ago

@mvastola Yes I would expect the sass-loader to be first, and for vanilla imports the url() is not needed... In fact I had never seen it (in my ignorance) in an @import statement before.

I am just checking, out of interest, what the reworkcss engine does with that @import url().

p.s. Any goose chase would be my fault for not understanding. Especially since you have an example project.

mvastola commented 6 years ago

Yeah, apparently it's totally optional. Ideally though, with or without the implicit url() it would be nice if resolve-url-loader handled it. That however, is an entirely separate issue. (Let me know if you want me to make a GH issue.) I'm going to rework my tests to bypass this. (I'd still be interested to hear what you find though.)

mvastola commented 6 years ago

Ok. Just pushed a new commit to https://github.com/mvastola/webpacker-issue-932/. Everything seems to be working now. Thanks so much for all your help thus far. You've been amazing!

Only oddity is the debug flag still seems to be doing nothing at all, but I imagine something in webpacker is catching/redirecting the output. In any case, that will be turned off.

Let me know what the verdict is on the import statement. If that is due to something irrelevant, we can ignore it. The odd thing was though that it seemed not only was resolve-url-loader not helping resolve the path of the @import url() but even when that path was correct without the need for resolve-url-loader, resolve-url-loader didn't seem to kick in to help fix paths within the imported file.

From here, if you could let me know what you think the release schedule of this patch might look like, I'll update the webpacker team. Also, now that you know the exact complexity of the patch, is there any chance it might be backported?

bholloway commented 6 years ago

@mvastola did you require this attempts feature to get working?

I suspect that your case would work with the latest distribution. In which case you would not need to change to webpacker.

I can confirm that sass-loader is ignoring (or at least passing though) the @import url() statement. Interestingly the rework "visitor" doesn't consider import statements but the date is parsed and I could make a non-standard "visitor" that includes them.

mvastola commented 6 years ago

I'm totally confused by your first two paragraphs. Could you clarify?

The attempts feature is needed to stop the deep searching of the directory and limit the paths checked to just relative the entry file and the file containing the url() directive. It was never really a question of resolve-url-loader not working (per se), but rather it causing a potential security hole (or at least acting in unintended ways) by exposing files that were not meant to be exposed.

I can confirm that sass-loader is ignoring (or at least passing though) the @import url() statement. Interestingly the rework "visitor" doesn't consider import statements but the date is parsed and I could make a non-standard "visitor" that includes them.

This is really it's own issue then IMHO and isn't specifically a webpacker concern. Would you like me to make one? I don't know too much about how the loaders work, but maybe sass-loader is ignoring things in this example because it is a .css file?

bholloway commented 6 years ago

I'm meaning that this is an opt in feature (unless I have coded wrong). If you can't activate debug it seems unlikely you are activating attempts... Unless, as you say, stdout is maybe redirected.

Can I confirm that you get unacceptable behaviour when you set attempts to a very large integer.

mvastola commented 6 years ago

Ahh. Perfectly reasonable thing for me to confirm. I'll get back to you in a couple.

mvastola commented 6 years ago

Okay, so I just tested this by appending .old onto both ./app/javascript/images/blue.png and ./app/javascript/bar/images/blue.png. This leaves the only file named blue.png residing in ./app/javascript/foo/images, which isn't one of the expected search paths.

Then, I ran rake assets:clobber assets:precompile under different conditions to see what would happen.

  1. Without changing anything (so the configuration still has { debug: true, attempts: 1 }: I'm actually seeing debug output now for some reason, but it's a bit odd, and some redirection of the output is certainly taking place, because of how everything is ordered: 
    
mvastola@vastdesk:~/src/bugs/webpacker-issue-932/app/javascript/bar/images$ rake assets:clobber assets:precompile
(in /home/mvastola/src/bugs/webpacker-issue-932)
I, [2017-10-23T18:05:11.290075 #3522]  INFO -- : Removed /home/mvastola/src/bugs/webpacker-issue-932/public/assets
Webpacker is installed 🎉 🍰
Using /home/mvastola/src/bugs/webpacker-issue-932/config/webpacker.yml file for setting up webpack paths
Removed webpack output path directory /home/mvastola/src/bugs/webpacker-issue-932/public/packs
yarn install v1.2.1
[1/4] Resolving packages...
success Already up-to-date.
Done in 1.18s.
I, [2017-10-23T18:05:14.241958 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-ea8ca43729dabd837f81720aa2306d5344b0c1930e9d7f1677532fa5c24072bd.js
I, [2017-10-23T18:05:14.242484 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-ea8ca43729dabd837f81720aa2306d5344b0c1930e9d7f1677532fa5c24072bd.js.gz
I, [2017-10-23T18:05:14.244096 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-f0d704deea029cf000697e2c0181ec173a1b474645466ed843eb5ee7bb215794.css
I, [2017-10-23T18:05:14.244257 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-f0d704deea029cf000697e2c0181ec173a1b474645466ed843eb5ee7bb215794.css.gz
I, [2017-10-23T18:05:14.250645 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/@rails/webpacker/lib/install/javascript/packs/application-e7f8c779987cf143adea07034e42daa9011b8e271378f6d21f489d0c4ba07ded.js
I, [2017-10-23T18:05:14.251116 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/@rails/webpacker/lib/install/javascript/packs/application-e7f8c779987cf143adea07034e42daa9011b8e271378f6d21f489d0c4ba07ded.js.gz
I, [2017-10-23T18:05:14.859328 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/express/lib/application-9232ebdb20ad39572e70fb9e29810e63dbb63b58f5f18617c7c2bc8bd28321b5.js
I, [2017-10-23T18:05:14.859558 #3522]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/express/lib/application-9232ebdb20ad39572e70fb9e29810e63dbb63b58f5f18617c7c2bc8bd28321b5.js.gz
Compiling…
Compilation failed:
Using rake 12.1.0
Using concurrent-ruby 1.0.5
Using i18n 0.9.0
Using minitest 5.10.3
Using thread_safe 0.3.6
Using tzinfo 1.2.3
Using activesupport 5.1.4
Using builder 3.2.3
Using erubi 1.7.0
Using mini_portile2 2.3.0
Using nokogiri 1.8.1
Using rails-dom-testing 2.0.3
Using crass 1.0.2
Using loofah 2.1.1
Using rails-html-sanitizer 1.0.3
Using actionview 5.1.4
Using rack 2.0.3
Using rack-test 0.7.0
Using actionpack 5.1.4
Using nio4r 2.1.0
Using websocket-extensions 0.1.2
Using websocket-driver 0.6.5
Using actioncable 5.1.4
Using globalid 0.4.0
Using activejob 5.1.4
Using mime-types-data 3.2016.0521
Using mime-types 3.1
Using mail 2.6.6
Using actionmailer 5.1.4
Using activemodel 5.1.4
Using arel 8.0.0
Using activerecord 5.1.4
Using public_suffix 3.0.0
Using addressable 2.5.2
Using bindex 0.5.0
Using bundler 1.16.0.pre.3
Using byebug 9.1.0
Using mini_mime 0.1.4
Using xpath 2.1.0
Using capybara 2.15.4
Using ffi 1.9.18
Using childprocess 0.8.0
Using coffee-script-source 1.12.2
Using execjs 2.7.0
Using coffee-script 2.4.1
Using method_source 0.9.0
Using thor 0.20.0
Using railties 5.1.4
Using coffee-rails 4.2.2
Using multi_json 1.12.2
Using jbuilder 2.7.0
Using rb-fsevent 0.10.2
Using rb-inotify 0.9.10
Using ruby_dep 1.5.0
Using listen 3.1.5
Using puma 3.10.0
Using rack-proxy 0.6.2
Using sprockets 3.7.1
Using sprockets-rails 3.2.1
Using rails 5.1.4
Using rubyzip 1.2.1
Using sass-listen 4.0.0
Using sass 3.5.2
Using tilt 2.0.8
Using sass-rails 5.0.6
Using selenium-webdriver 3.6.0
Using spring 2.0.2
Using spring-watcher-listen 2.0.1
Using sqlite3 1.3.13
Using turbolinks-source 5.0.3
Using turbolinks 5.0.1
Using uglifier 3.2.0
Using web-console 3.5.1
Using webpacker 3.0.2 from https://github.com/mvastola/webpacker.git (at master@db41897)
Bundle complete! 17 Gemfile dependencies, 74 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.

resolve-url-loader: images/blue.png /home/mvastola/src/bugs/webpacker-issue-932/app/javascript/bar NOT FOUND (INTERRUPTED)

resolve-url-loader: images/blue.png /home/mvastola/src/bugs/webpacker-issue-932/app/javascript/bar NOT FOUND (INTERRUPTED) Hash: ab05d6ad9855453f773d Version: webpack 3.7.1 Time: 3085ms Asset Size Chunks Chunk Names application-97f332c408a06c382a8d.js 8.4 kB 0 [emitted] application manifest.json 68 bytes [emitted]
[0] ./app/javascript/packs/application.js 29 bytes {0} [built] [1] ./app/javascript/application.scss 4.26 kB {0} [built] [failed] [1 error] [2] ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"/home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss 233 bytes [built]

ERROR in ./app/javascript/application.scss Module build failed: ModuleNotFoundError: Module not found: Error: Can't resolve './images/blue.png' in '/home/mvastola/src/bugs/webpacker-issue-932/app/javascript' at factoryCallback (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/Compilation.js:274:40) at factory (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:235:20) at resolver (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:60:20) at asyncLib.parallel.e (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:127:20) at /home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:3861:9 at /home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:421:16 at iteratorCallback (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:996:13) at /home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:906:16 at /home/mvastola/src/bugs/webpacker-issue-932/node_modules/async/dist/async.js:3858:13 at resolvers.normal.resolve (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/webpack/lib/NormalModuleFactory.js:119:22) at onError (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:65:10) at loggingCallbackWrapper (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19) at runAfter (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:158:4) at innerCallback (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:146:3) at loggingCallbackWrapper (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19) at next (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:252:11) at /home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/UnsafeCachePlugin.js:40:4 at loggingCallbackWrapper (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19) at runAfter (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:158:4) at innerCallback (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:146:3) at loggingCallbackWrapper (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19) at next (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:252:11) at innerCallback (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:144:11) at loggingCallbackWrapper (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19) at next (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:249:35) at resolver.doResolve.createInnerCallback (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/DescriptionFilePlugin.js:44:6) at loggingCallbackWrapper (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19) at afterInnerCallback (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/Resolver.js:168:10) at loggingCallbackWrapper (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/enhanced-resolve/lib/createInnerCallback.js:31:19) at next (/home/mvastola/src/bugs/webpacker-issue-932/node_modules/tapable/lib/Tapable.js:252:11)

ERROR in ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"/home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss Module not found: Error: Can't resolve './images/blue.png' in '/home/mvastola/src/bugs/webpacker-issue-932/app/javascript' @ ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"/home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss 6:56-84 @ ./app/javascript/application.scss @ ./app/javascript/packs/application.js Child extract-text-webpack-plugin node_modules/extract-text-webpack-plugin/dist node_modules/css-loader/index.js??ref--5-2!node_modules/postcss-loader/lib/index.js??ref--5-3!node_modules/resolve-url-loader/index.js??ref--5-4!node_modules/sass-loader/lib/loader.js??ref--5-5!app/javascript/application.scss: [0] ./node_modules/css-loader?{"minimize":false}!./node_modules/postcss-loader/lib?{"sourceMap":true,"config":{"path":"/home/mvastola/src/bugs/webpacker-issue-932/.postcssrc.yml"}}!./node_modules/resolve-url-loader?{"debug":true,"attempts":1,"absolute":false,"sourceMap":false,"engine":"rework","fail":false,"silent":false,"keepQuery":false,"root":null,"includeRoot":false}!./node_modules/sass-loader/lib/loader.js?{"sourceMap":true}!./app/javascript/application.scss 233 bytes {0} [built]

  1. Changing ./node_modules/@rails/webpacker/package/loaders/style.js to set attempts to 999 (leaving debug set to true): Everything compiles successfully with no errors and no debug output: 
    mvastola@vastdesk:~/src/bugs/webpacker-issue-932$ rake assets:clobber assets:precompile
I, [2017-10-23T18:08:21.304861 #13610]  INFO -- : Removed /home/mvastola/src/bugs/webpacker-issue-932/public/assets
Webpacker is installed 🎉 🍰
Using /home/mvastola/src/bugs/webpacker-issue-932/config/webpacker.yml file for setting up webpack paths
Removed webpack output path directory /home/mvastola/src/bugs/webpacker-issue-932/public/packs
yarn install v1.2.1
[1/4] Resolving packages...
success Already up-to-date.
Done in 1.20s.
I, [2017-10-23T18:08:26.469965 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-ea8ca43729dabd837f81720aa2306d5344b0c1930e9d7f1677532fa5c24072bd.js
I, [2017-10-23T18:08:26.470486 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-ea8ca43729dabd837f81720aa2306d5344b0c1930e9d7f1677532fa5c24072bd.js.gz
I, [2017-10-23T18:08:26.472982 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-f0d704deea029cf000697e2c0181ec173a1b474645466ed843eb5ee7bb215794.css
I, [2017-10-23T18:08:26.473155 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/application-f0d704deea029cf000697e2c0181ec173a1b474645466ed843eb5ee7bb215794.css.gz
I, [2017-10-23T18:08:26.490296 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/@rails/webpacker/lib/install/javascript/packs/application-e7f8c779987cf143adea07034e42daa9011b8e271378f6d21f489d0c4ba07ded.js
I, [2017-10-23T18:08:26.490508 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/@rails/webpacker/lib/install/javascript/packs/application-e7f8c779987cf143adea07034e42daa9011b8e271378f6d21f489d0c4ba07ded.js.gz
I, [2017-10-23T18:08:27.090115 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/express/lib/application-9232ebdb20ad39572e70fb9e29810e63dbb63b58f5f18617c7c2bc8bd28321b5.js
I, [2017-10-23T18:08:27.090323 #13610]  INFO -- : Writing /home/mvastola/src/bugs/webpacker-issue-932/public/assets/express/lib/application-9232ebdb20ad39572e70fb9e29810e63dbb63b58f5f18617c7c2bc8bd28321b5.js.gz
Compiling…
Compiled all packs in /home/mvastola/src/bugs/webpacker-issue-932/public/packs
mvastola commented 6 years ago

So anyway it seems everything is working. Also, it seems something is being done inside webpacker such that the debug output is only printed if the compile fails. (Which I guess is to make for cleaner output).

Finally, I have no idea why this is happening, but the debug output is showing the same path being tried (and failing) twice, rather than two separate paths (or just one path, if that should be the case).

bholloway commented 6 years ago

I am not sure why twice. However that is the debug output I would expect for attempts=1.

Is that not the correct location of those .png files, why would it be NOT FOUND I wonder. 🤔

mvastola commented 6 years ago

I think you missed.

Okay, so I just tested this by appending .old onto both ./app/javascript/images/blue.png and ./app/javascript/bar/images/blue.png.

bholloway commented 6 years ago

Just for the record, is it non-performant with the existing version. Maybe you answered this but I missed it in your post.

Do you need this feature immediately for your use-case? (Stated another way) will the old version work or do you need back-port right now?

bholloway commented 6 years ago

"I think you missed."

Yes I did, sorry I get it now.

The files are not there, and there is no costly/inaccurate search for them.

I am thinking that, regardless of whether the old releases will work for you, there is a real concern that you may accidentally pull in the wrong asset. And that you want attempts=1 so that it will fail hard.

I am thinking that this is the better (default) way to be. Maybe there should be a major release that adds attempts=1 and people need to opt in the the old (non-robust) behaviour.

EDIT - in the mean time we could consider back-port of attempts=0 as a minor release

mvastola commented 6 years ago

Sorry if this wasn't clear or easy to understand earlier. Let me see if I can explain.

The underlying issue we (webpacker) has, and the reason for starting this GitHub issue is that (as you just point out) the wrong asset can be pulled due to an exhaustive search if, say, someone typos the path inside a url but it matches another file in a different directory.

The impact of this unexpected behavior depends on the application and could be just a minor hard-to-find glitch, or it could represent a very problematic data leak (if, say, the path is resolved to a file named double-super-secret-non-public-directory-do-not-show-to-anyone/image.gif). On top of that is the impact in terms of CPU cycles, hard disk cost, etc.

So to answer your question, resolve-url-loader isn't non-performant per se, but it is -- for the purposes of this discussion -- broken in its current release. For it to be "fixed", it at least needs the ability to set attempts=1.

I wholly agree that attempts=1 is the better (and perhaps only) way to be, but the choice of default doesn't really impact us since (as demonstrated), we can pass in options easily.

bholloway commented 6 years ago

So I think this is good. I just need to change the readme.md and I will merge this.

The back-port would be a following step, but so long as webpacker are in agreement I think it should be relatively easy.

mvastola commented 6 years ago

SGTM. I just gave webpacker a bit of a longer rundown of what's been going on because I remember seeing that none of them were following #69.

bholloway commented 6 years ago

Any input on the docs @mvastola or will I merge this sucker?

mvastola commented 6 years ago

:+1: on the docs.

mvastola commented 6 years ago

Actually, give me a sec. I want to checkout bootstrap 3. It'd be nice if we could throw in directions on how to use it in conjunction with attempts=1. (The latest bootstrap has no fonts.)

mvastola commented 6 years ago

Ok. So FYI, bootstrap-sass works out of the box with attempts=1 with no further changes. Everything compiled cleanly and used the right paths.

I tested this by doing yarn add jquery bootstrap-sass and putting this in the entry js file:

import 'jquery/src/jquery'
import 'bootstrap-sass/assets/javascripts/bootstrap.js'
import 'bootstrap-sass/assets/stylesheets/_bootstrap.scss'

So I would suggest tweaking the README.md very slightly in the following ways.

  1. Change (it contains a typo anyway, now that I read it closely): However in some cases there is no immediate match (*cough* bootstrap *cough*) and we so to However in cases where there is no immediate match, we.
  2. Immediately after this text is the phrase start searching. I would link it to the File search "magic" section.
  3. In the File search "magic" section, immediately after It is less relevant now, I would add, in parentheses: (the broken packages *cough* bootstrap *cough* seem to be fixed).
bholloway commented 6 years ago

Good call, I went with most of this feedback.

I also took a good look at what "magic" I had documented. This was done before the debug option. After looking at the debug output I realised it is not what had been assumed.

The sooner we can deprecate the "magic", the better.

mvastola commented 6 years ago

Sounds good. Only remaining feedback I have is now these two sentences in the README.md are almost identical. Not sure if you meant to do that..

bholloway commented 6 years ago

It was intentional, but a bit silly I guess.

I am going to have to rewrite the readme.md for 3.0.0 with usage for Webpack 2. I value your feedback, so if you are amenable I will ping you on that review.

mvastola commented 6 years ago

Totally amenable.

Anywho, this is all good to merge into the v3 branch as far as I'm concerned.

bholloway commented 6 years ago

This PR is on top of breaking changes feature/multiple-engines which meant it could not be immediately released. A separate PR #71 has back-ported these changes to master for immediate release.

Following that I rebased feature/multiple-engines to master. That then caused this branch to have zero changes with respect to its base. Github has therefore closed this PR as complete.

bholloway commented 6 years ago

Published as 2.2.0