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

addURLDependency: use always relative path #2518

Merged
merged 1 commit into from Jan 7, 2019
Merged

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Jan 6, 2019

↪️ Pull Request

Fixes #1801
Previously rejected implementation: #1809

💻 Examples

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

Running parcel with the two bundles.css as entry points, in the output

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

a/bundle.css would contain url("foo.d7b28491.png") (should be "../foo")

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@DeMoorJasper
Copy link
Member

@mischnic just fyi the previous PR you linked just got closed because master had been force-pushed and Github automatically closed all open PRs, it had not been rejected.

@mischnic
Copy link
Member Author

mischnic commented Jan 7, 2019

just fyi the previous PR you linked just got closed because master had been force-pushed and Github automatically closed all open PRs, it had not been rejected.

Didn't compare the dates 😄 . But @devongovett wrote:

I think it already gives a relative path. This makes the output more portable depending on where you move it to. Why does this not work?

The previous PR added the public url, this makes the relative path correct.

@rhurstdialpad
Copy link
Contributor

rhurstdialpad commented Feb 27, 2019

@DeMoorJasper @devongovett This broke our build.

We have --public-url set to /static/compiled/single.
Our fonts are in /fonts and our CSS is in css/single.

Currently, our CSS files that reference font files are ending up with a URL to each font file that includes the public URL and a path that seems to be the resolved path from the root dir and the relative path from the CSS file to the font file. The path from the CSS directory to the fonts directory is ../../fonts. So in our case the font urls all look something like /static/compliled/single/[dir outside project]/[dir outside project]/[font file].xxxxx.woff. Obviously, this is totally wrong, and as the dev server 404s when trying to load the fonts.

This PR is cool, but something is not quite right here.

@mischnic
Copy link
Member Author

We have --public-url set to /static/compiled/single.
Our fonts are in /fonts and our CSS is in css/single.

Is that second line referring to the url or your folder structure during development?
Could you please give an overview how both look like? Is this correct?
File system:

─ MyProject
  ├─ fonts
  │  └─ *.woff
  └─ css
     └─ single
        └─ *.css

Wanted output structure:

─ /static/compiled/single/
  ├─ *.woff
  └─ *.css

What does dir outside project mean exactly?

Otherwise, a very minimal reproduction would be helpful.

mischnic added a commit that referenced this pull request Mar 7, 2019
Fix real core issue of #2518
@rhurstdialpad
Copy link
Contributor

That looks about right, with the exception of the output structure. /static/compiled/single/ is the path I want the dev server to serve the assets on. My dist folder is the default.

When I say folder outside the project I mean just that. If my project is at /Users/Robert/Developer/MyProject, then the path is /static/compliled/single/Robert/Developer/[font file].xxxxx.woff.

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

Successfully merging this pull request may close these issues.

None yet

4 participants