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

[v2.1.1] breaks resolve-url-loader #340

Closed
jussikinnula opened this issue Feb 26, 2018 · 15 comments
Closed

[v2.1.1] breaks resolve-url-loader #340

jussikinnula opened this issue Feb 26, 2018 · 15 comments

Comments

@jussikinnula
Copy link

When upgrading to 2.1.1 version of postcss-loader, file-loader and url-loader functionality is broken (e.g. using require() in JavaScript files or url() in CSS files).

The error message:

[0] WARNING in ./node_modules/css-loader??ref--3-1!./node_modules/resolve-url-loader??ref--3-2!./node_modules/postcss-loader/lib??ref--3-3!./node_modules/stylus-loader??ref--3-4!./app/App/index.styl
[0] (Emitted value instead of an instance of Error)   resolve-url-loader cannot operate: CSS error
[0]   ENOENT: no such file or directory, scandir '/Users/xxx/dev/app/App/app/App'
[0]   at Object.fs.readdirSync (fs.js:904:18)
[0]  @ ./app/App/index.styl 4:14-254
[0]  @ ./app/App/index.tsx
[0]  @ ./app/index.tsx

The problem seems to be that stylus-loader the path wrong. It should be /Users/xxx/dev/app/App (not /Users/xxx/dev/app/App/app/App).

On this case the index.styl contains:

.App
  background-image url('./foo.png')

On Webpack configuration, context is set like:

context: path.resolve(__dirname, './'),

Downgrading to postcss-loader 2.1.0 gets everything working again.

@jussikinnula
Copy link
Author

Manually readding to: file, to options (ultimately reverting the PR #339 fix) resolves the issue:
https://github.com/postcss/postcss-loader/blob/master/lib/index.js#L105-L112

@Demivan
Copy link

Demivan commented Feb 26, 2018

Have same issue with resolve-url-loader.
resolve-url-loader uses SourceMapConsumer.originalPositionFor from mozilla/source-map. Path resolved by that method is now wrong:

{ source: 'D:/Projects/test/node_modules/video.js/dist/D:/Projects/test/node_modules/video.js/dist/video-js.css',
  line: 17,
  column: 2,
  name: null }

with version 2.1.0:

{ source: 'D:/Projects/test/node_modules/video.js/dist/video-js.css',
  line: 17,
  column: 2,
  name: null }

@alexander-akait
Copy link
Member

Move postcss-loader above resolve-url-loader

@alexander-akait
Copy link
Member

Also resolve-url-loader use rework (which deprecated and very buggy)

@Demivan
Copy link

Demivan commented Feb 26, 2018

Yeah, I know :). But it does not change that mozilla/source-map is now returning wrong path. It could break other loaders too.

@alexander-akait
Copy link
Member

/cc @michael-ciniawsky

@jussikinnula
Copy link
Author

After changing the order (moving postcss-loader above resolve-url-loader) gets rid of the error.

I understand the fact that many webpack loaders are buggy, but for now I haven't been able to resolve resolve importing/requiring a Stylus file inside a Stylus file - without using resolve-url-loader. Without it, the imported Stylus files are looking url() inside the same directory where the file requiring another is.

@alexander-akait
Copy link
Member

@jussikinnula we can just rewrite resolve-url-loader on postcss and release this to avoid problem in url for scss/stylus and etc

@jussikinnula
Copy link
Author

@evilebottnawi, that would be superb. It would simplify our life of bundling a lot of reusable React components in component libraries :-)

@Demivan
Copy link

Demivan commented Feb 26, 2018

There is already pending pull request to use postcss in resolve-url-loader

@Demivan
Copy link

Demivan commented Feb 26, 2018

but resolve-url-loader as postcss plugin would be just great

@michael-ciniawsky michael-ciniawsky changed the title PR #339 breaks file-loader/url-loader functionality [v2.1.1] breaks resolve-url-loader functionality Feb 27, 2018
@michael-ciniawsky michael-ciniawsky changed the title [v2.1.1] breaks resolve-url-loader functionality [v2.1.1] breaks resolve-url-loader Feb 27, 2018
@michael-ciniawsky
Copy link
Member

Sry, I didn't think of resolve-url-loader when merging #339 :). The fact is that

  1. resolve-url-loader is basically a hack in general
  2. This should be fixed in/guarded by resolve-url-loader itself
  3. The use case for resolve-url-loader can be implemented in a cleaner way
  4. It forces other loaders like e.g postcss-loader to produce malformed source maps then

The change should not have landed this way (as a patch) in retro spec, but since resolve-url-loader should/could be fixed and changing the loader order seems to work aswell, I'm not going to revert this change for now

@bholloway
Copy link

It seems like the source-map outgoing from postcss-loader is corrupted before it enters resolve-url-loader, at least on windows.

See analysis here. Please let me know your thoughts.

@julkue
Copy link

julkue commented Mar 5, 2018

@michael-ciniawsky Coming here since I had this issue and debugged this with the help of @bholloway's analysis (see above). I could only fix it with #340 (comment). However, it took a while to come here, so this can't be a solution for the mass. I'd highly appreciate if you and @bholloway could please coordinate a solution. The points you've written may be right, but they don't offer a solution to the problem. To me it feels like: "not my problem, I don't care".

@bholloway
Copy link

There could be a couple of issues going on here. But for Windows I am suggesting there is probably a large problem.

After some digging it looks like mozilla/source-map (used by postcss) has considered paths to be URIs for at least 2 years. Hence this silly concatenation on windows.

I note that postcss state that "you should always set 'to' to generate correct source maps". This could mean a lot of things but I would bet this avoids the Windows issue most of the time.

(P.S. I use postcss-loader myself and appreciate all your work)

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

No branches or pull requests

6 participants