-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix RawAsset loading. #160
Conversation
src/assets/RawAsset.js
Outdated
@@ -6,8 +6,11 @@ class RawAsset extends Asset { | |||
load() {} | |||
|
|||
generate() { | |||
const pathToAsset = JSON.stringify( | |||
path.join(this.options.publicURL, this.generateBundleName()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this cause a \
to be used on windows instead of /
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you probably want url.resolve
https://nodejs.org/api/url.html#url_url_resolve_from_to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the heads up. On it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I had misunderstood how modules are loaded. Thanks for the review !
src/assets/RawAsset.js
Outdated
@@ -6,8 +6,11 @@ class RawAsset extends Asset { | |||
load() {} | |||
|
|||
generate() { | |||
const pathToAsset = JSON.stringify( | |||
path.join(this.options.publicURL, this.generateBundleName()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you probably want url.resolve
https://nodejs.org/api/url.html#url_url_resolve_from_to
Not sure if this was supposed to fix images not loading in dev environment but I'm still having this issue on v1.2.0 |
@drejohnson I think you should make a new issue or comment on one of the closed ones if they pertain to the bug your experiencing. We definitely need to look into this if it’s still a problem! Sent with GitHawk |
* Prepending raw asset generated bundle name with publicURL. * Working code. Failing tests. * Fix whoops. * Back to working. * Resolve Raw Asset URL instead of path.
* Prepending raw asset generated bundle name with publicURL. * Working code. Failing tests. * Fix whoops. * Back to working. * Resolve Raw Asset URL instead of path.
Hello !
Fixes :
jaredpalmer/react-parcel-example#6 #96 #186
Problem :
In RawAsset, the generated bundle name was not prepended by the public url.
This solution :
Prepend RawAsset generated bundle name with options.publicURL (defaults to dist). Updated tests to reflect the change.
Tests :
Passing. I also, tested my fork against https://github.com/jaredpalmer/react-parcel-example and it works in both dev and prod mode.
Thanks for the great work on Parcel ! 👍
Please feel free to close if this code affects other parts negatively.