Skip to content

Empty string is rewritten as './' in urlToRequest #80

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

Closed
darrenscerri opened this issue Mar 22, 2017 · 6 comments
Closed

Empty string is rewritten as './' in urlToRequest #80

darrenscerri opened this issue Mar 22, 2017 · 6 comments

Comments

@darrenscerri
Copy link
Contributor

darrenscerri commented Mar 22, 2017

Related issue: webpack-contrib/css-loader#463

Calling urlToRequest with an empty string resolves to ./. This behaviour should not be allowed, or rather keep as an empty string and let Webpack handle it.

@jhnns
Copy link
Member

jhnns commented Mar 23, 2017

Mhmm, I think the current behavior is correct. CSS does not distinguish between url('file.jpg') and url('./file.jpg'). That's why an empty string '' should be equivalent to './'. Could you describe your use-case? Why do you have an empty url() statement—and how is webpack supposed to handle an empty request?

@jhnns
Copy link
Member

jhnns commented Mar 23, 2017

Ah ok ... this was an error in the first place. What error does webpack report in this case?

@darrenscerri
Copy link
Contributor Author

Webpack doesn't report any errors but proceeds to trying to require './ even if it's a JS file.

The original issue was here: facebook/create-react-app#1875. The url('') was a mistake but lead to a very hard-to-find bug.

Webpack currently warns if somebody does a require('') (webpack/webpack#2006).

Maybe we should warn if somebody tries to rewrite ''?

@jhnns
Copy link
Member

jhnns commented Mar 23, 2017

Yeah, I totally see that. While transforming '' to './' is theoretically correct, it is most likely an error.

I thought about throwing an error if an empty url is passed, but that wouldn't be very pleasant to use and it would be a breaking change since the call needs to be wrapped in try/catch.

Maybe it's best to just keep it as it is and let webpack handle it correctly (as you suggested)... not the most beautiful solution 😞, but it would probably work.

@jhnns
Copy link
Member

jhnns commented Mar 23, 2017

@darrenscerri Would you be up to a pull-request? Should be a small change...

@darrenscerri
Copy link
Contributor Author

@jhnns Added pull-request

jhnns pushed a commit that referenced this issue Mar 24, 2017
Prior to this commit, `urlToRequest` rewrote an empty URL `''` to `'./'` which is correct from a theoretical point of view since CSS files do not distinguish between relative paths starting with ./ or not (like CommonJS does). However, an empty URL is most likely an error which webpack should warn about. If we rewrite the empty string to ./, webpack won't warn against this anymore because requiring ./ is valid.

Related discussion: #80
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

2 participants