-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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.