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

Wrong relative path to URL assets when bundle lives in subdirectory #1801

Closed
kalfheim opened this issue Jul 26, 2018 · 5 comments
Closed

Wrong relative path to URL assets when bundle lives in subdirectory #1801

kalfheim opened this issue Jul 26, 2018 · 5 comments

Comments

@kalfheim
Copy link

🐛 Bug Report

When two bundles in different directories with the same file name are generated, the directory structure is reflected in the output. This causes an issue with URL dependencies because their paths are relative to the current directory.

🎛 Configuration

Given the following directory structure:

assets
├── a
│   └── bundle.css
├── b
│   └── bundle.css
└── img
    └── foo.png

With the contents of assets/a/bundle.css being:

body {
    background-image: url('../img/foo.png');
}

And the following Parcel command:

parcel build --no-cache --no-source-maps 'assets/*/bundle.css'

The following directory tree is correctly generated:

dist
├── a
│   └── bundle.css
├── b
│   └── bundle.css
└── foo.d7b28491.png

So far so good. However...

🤔 Expected Behavior

I would expect the contents of dist/a/bundle.css to be either:

body{background-image:url(../foo.d7b28491.png)}

Or:

body{background-image:url(/foo.d7b28491.png)}

😯 Current Behavior

The contents of dist/a/bundle.css is currently:

body{background-image:url(foo.d7b28491.png)}

Which means the browser will request /a/foo.png, but the file is located at the web root – not in the "a" directory.

💁 Possible Solution

I'm really not familiar enough with Parcel to make an educated recommendation as to how is should be fixed, but I'm guessing the best way to fix the issue is by somehow making calculating the relative path to the URL asset from the bundle. So that (using the example code above), the output URL would be url(../foo.[md5].png)

Another possible solution is to join the URL with the publicURL option. This could be achieved by modifying the Asset#addURLDependency() method's return value, like so:

+ const urlJoin = require('./utils/urlJoin');

...

- return URL.format(parsed);
+ return urlJoin(this.options.publicURL, URL.format(parsed));

This method seems to work, but as said before, I'm not familiar enough with Parcel to know if it's a viable solution. It does require modifying some tests, which indicates it could be a breaking change. Would love some feedback.

I have made a commit and will definitely create a PR if the solution is approved.

🌍 Your Environment

Software Version(s)
Parcel 1.9.7
Node 10.7.0
npm/Yarn Yarn 1.7.0
Operating System macOS 10.13.6
@DeMoorJasper
Copy link
Member

I think your solution is the correct one in this case. The result should always have the publicUrl as a prefix

@kalfheim
Copy link
Author

kalfheim commented Jul 27, 2018

@krotovic
Copy link

What is the status of this issue? I would like to switch from webpack to parcel but this is really big problem for me. Is there any workaround for the time before any fix?

@Sneezoo
Copy link

Sneezoo commented Jan 17, 2019

Hi I am currently having the same problem here and I would like to know when the next version with this fix is released.

@kevincox
Copy link
Contributor

I would really like to have this feature as well. One of the key benefits of parcel is that it generates unique file paths for assets. This prevents version mismatches and allows aggressive caching. However it is very difficult to set caching headers for these assets because there is no easy pattern to select them with. If there was a directory for non-entrypoints then it would be easy to set cache forever headers on anything in that directory.

The only workaround I can think of right now is setting --public-url=assets/$buildid and then mapping that directory back into the root using rewriting which is quite messy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants