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

Properly resolve url(...) in style strings #182

Closed
satya164 opened this issue Nov 30, 2017 · 4 comments · Fixed by #248
Closed

Properly resolve url(...) in style strings #182

satya164 opened this issue Nov 30, 2017 · 4 comments · Fixed by #248
Assignees
Labels
bug 🐛 Issue is a confirmed bug status: has PR 😍 Issue has opened PR

Comments

@satya164
Copy link
Member

What's broken

Say you have the following file structure:

├── .linaria-cache
├── src
│   ├── components
│   │   └── App.js
│   └── assets
│       └── bat.png
└── .babelrc

Now you write following in App.js:

const logo = css`
  background: url(../assets/bat.png);
`;

This won't work, because the generated CSS file will be at .linaria-cache/src/components/App.css, and when resolving ../assets/bat.png, webpack will look relative to the the generated file.

How to fix

We can write a stylis plugin which replaces relative paths in url with the correct path relative to the generated file.

@satya164 satya164 added bug 🐛 Issue is a confirmed bug good first issue 😊 It is a good issue for new comers labels Nov 30, 2017
@zamotany
Copy link
Contributor

I think we can calculate the correct path without stylis

@satya164
Copy link
Member Author

@zamotany how? the url statements will be resolved by webpack and style-loader. we don't have any control over how it's resolved.

@zamotany
Copy link
Contributor

But with this:

const logo = css`
  background: url(${require(../assets/bat.png)});
`;

we can calculate the correct path.

Tho you're right, the solution with stylis will work for both cases

@morajabi
Copy link

It's interesting! Do you want me to give this a hand? 🙌

@zamotany zamotany added this to the 0.6.0 milestone Jan 8, 2018
@zamotany zamotany added the status: has PR 😍 Issue has opened PR label Jan 16, 2018
@zamotany zamotany self-assigned this Jan 16, 2018
@zamotany zamotany removed the good first issue 😊 It is a good issue for new comers label Jan 16, 2018
@zamotany zamotany removed this from the 0.6.0 milestone Sep 20, 2018
satya164 added a commit that referenced this issue Oct 12, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 12, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 12, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 12, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 12, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 12, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 14, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 14, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 14, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
satya164 added a commit that referenced this issue Oct 15, 2018
This commit adds a plugin to replace relative paths inside url(..) expressions to be relative to the output file so that they can be resolved properly with css-loader.

It also changes the output folder to be 'node_modules/.linaria-cache' like before.

Inspired by #195
Fixes #182
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Issue is a confirmed bug status: has PR 😍 Issue has opened PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants