Skip to content

Reference CSS resources using relative paths #530

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Reference CSS resources using relative paths #530

wants to merge 2 commits into from

Conversation

asselin
Copy link

@asselin asselin commented Feb 18, 2017

When referencing resources (e.g. fonts) in a CSS file, absolute paths are put into the generated CSS file. If assetsPath is absolute (e.g. '/'), this is OK, but if it's relative (e.g. './'), production builds won't be able to find the assets (because of the absolute rather than relative path).

I created a repo to discuss this issue: https://github.com/asselin/vue-webpack-font-load-bug. The project pulls in foundation-icons, which references a font in the CSS. I added 1 icon under "Essential links" in Hello.vue.

Branch build_with_relative_assetsPath changes the assetsPath to './', and showcases the issue. The font file won't be found if you build for production because the URL put into the CSS is static/fonts/foundation-icons.a188c2f.woff (vs. /static/fonts/foundation-icons.a188c2f.woff if assetsPath is '/').

Branch relative_assets_path_with_url-loader_change applies my fix, which changes the URL in the CSS file to be ../fonts/foundation-icons.a188c2f.woff.

Branch Base_template_with_url-loader_change shows that my change with assetsPath set to '/' still works for dev and prod builds.

This was referenced Feb 21, 2017
Open
Open
@LinusBorg
Copy link
Contributor

Wouldn't that break when we use vue-router in history mode? as soon as we navigate to some route, the relative path be wrong...

LinusBorg
LinusBorg suggested changes Feb 21, 2017
View reviewed changes
Copy link
Contributor

@LinusBorg LinusBorg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I "request changes" to block this from merging until open questions about side effects with vue-router are evaluated.

@LinusBorg
Copy link
Contributor

Related PR: #541

@asselin
Copy link
Author

asselin commented Feb 21, 2017

@LinusBorg I can give it a test, but I don't think it will break, because the font's URL is relative to the CSS file-- assuming you can load the CSS file, the font's relative URL should be valid.

@LinusBorg
Copy link
Contributor

because the font's URL is relative to the CSS file

Oh, right. Still wanna test it to be sure, webpack can be ticky with paths and stuff :D

@LinusBorg LinusBorg self-assigned this Feb 22, 2017
@asselin
Copy link
Author

asselin commented Feb 22, 2017

I tested with vue-router history mode, and it worked fine, no issues. @LinusBorg, would it be possible to merge this one please?

@LinusBorg
Copy link
Contributor

I'm currently setting up a test with lazy-loading routes, asking for a little more patience. thanks :)

@LinusBorg
Copy link
Contributor

LinusBorg commented Feb 25, 2017 •
edited
Loading

So I tested it, and it's like I thought: When using history mode for the router, CSS stops working with this change for these scenarios:

  • directly visiting a subroute (refresh the browser while on a subroute) (this also breaks in dev, because the relative link to the css file in the <head> is wrong now)
  • using lazy-loaded routes ("only" breaks in production, because of the following:)

When we are at /test1/subtest1, then the SubTest1 component injects its CSS into the head in a <style>tag, with tese relative paths, e.g. ../fonts - which now points to /test1/fonts - which doesn't exist.

So, to summarize:

  • If we leave absolute paths for assets, those don't work with relative publicPath in webpack if the app is served from a subdirectory.
  • If we change it to relative paths, then lazy-loaded route components with vue-router produce invalid CSS paths to assets.

Neither are optimal. But if I have to choose, I would vote to stay with absolute paths (for both assets and publicPath). Why?

  • With absolute paths, every app can be made to work in any subdirectory with a change of the assetsPublicPAth to match the subdirectory and a rebuild.
  • But with relative paths, Someone working at an app that is working great in dev, would have to somehow switch over from relative to absolute paths as soon as he wants to use lazy-loaded routes (which is a very common usecase for any bigger app).

Unless we find a way to make the second scenario much more user-friendly than it is now, I think we have to stay with absolute paths, or find a way to offer both ways as an easy to switch configuration option.

@asselin
Copy link
Author

asselin commented Mar 3, 2017

@LinusBorg When I tested history mode with relative paths, I didn't run into any issues, including reloading on a subroute. Note that for dev, I left the assetsPublicPath as '/', so there was no issue there.

I haven't tried lazy loaded routes-- do you happen to have a snippet of code or a github repo handy with your test (I'd like to see it in action to see if I can come up with any solutions). If not, no worries, I'll code something up.

Assuming it's an issue we can't get around, based on your comment, are you saying that you'd be OK with a PR that adds an additional question (e.g. Do you want to use an absolute or relative base path?) and then configures the template accordingly (webpack config, assetsPublicPath, etc.)?

Thanks!

@LinusBorg
Copy link
Contributor

have the repo locally, will upload it to GH tonight.

@asselin
Copy link
Author

asselin commented Mar 6, 2017

@LinusBorg Gentle reminder, when you have time, to upload your GH repo. Thanks!

@LinusBorg
Copy link
Contributor

Wanted to, but had trouble to reliably replictae the scenarios. Either way, relative paths seem to be brittle at best.

Will probably upload the repo tonight anyway - I hop you can find your way around the codebase with a few pointers in the readme.

@LinusBorg
Copy link
Contributor

LinusBorg commented Mar 7, 2017 •
edited
Loading

So here's the repo: https://github.com/LinusBorg/webpack-template-test-relative-paths

Edit: renamed the repo to remove stupid typos :D

@asselin
Copy link
Author

asselin commented Apr 19, 2017

@LinusBorg Apologies for taking so long to get back on this one (client work!).

I just sent you a PR to the repo you sent me above. I think the reason you were seeing an issue with reloads is because you had assetsPublicPath set to './' with history mode turned on. When I set it to '/', it worked fine.

Also note that this PR only affects the URLs for Fonts, and since fonts are specified in CSS files, their URL is relative to the CSS file, not relative to index.html.

Thanks!

@LinusBorg LinusBorg mentioned this pull request May 4, 2017
Closed
@LinusBorg LinusBorg removed their assignment Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LinusBorg LinusBorg LinusBorg requested changes

@aladdin-add aladdin-add aladdin-add approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@asselin @LinusBorg @aladdin-add