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

add relative_url_root to webpacker config #1236

Merged
merged 2 commits into from
Feb 24, 2018

Conversation

ipmsteven
Copy link
Contributor

@ipmsteven ipmsteven commented Jan 30, 2018

This PR will set WEBPACKER_RELATIVE_URL_ROOT as ActionController::Base.relative_url_root which partially solve this issue #1106

would you mind taking a look? @gauravtiwari
Thanks

@brunoOSI
Copy link

Please add this in, it is a bummer that you only can ship precompiled asset paths. It should allow some configuration!

@gauravtiwari
Copy link
Member

Thanks @ipmsteven for this. Sorry for the delay.
I think this requires a bit more wiring to function correctly like appending relative root to publicPath. Will take a look at this soon.

@ipmsteven
Copy link
Contributor Author

ipmsteven commented Feb 14, 2018

Hi @gauravtiwari

You are right, this PR only make relative_url_root available during webpack compile time. A bit more wiring need to be done to construct publicPath that respect relative_url_root.
I am thinking we could start with only exposing relative_url_root info and delegate the plumbing work to the user of webpacker for now.

so with that being said above, If you agree with the proposal of adding WEBPACKER_RELATIVE_URL_ROOT to webpacker compile environment,

we could either
Plan 1:

  1. merge this PR that only ships WEBPACKER_RELATIVE_URL_ROOT first
  2. have separate PR to do the wiring work as you mentioned above

OR

Plan 2: Have one PR handle both 1 and 2

I am fine with either one, what do you think? 😃

@gauravtiwari gauravtiwari merged commit f01102a into rails:master Feb 24, 2018
@gauravtiwari
Copy link
Member

gauravtiwari commented Feb 24, 2018

Thanks and sorry it took a while to get this one merged. I will take a look at part 2.

@ipmsteven ipmsteven deleted the addRelativeUrlRootEnvVar branch February 25, 2018 03:49
@ipmsteven
Copy link
Contributor Author

ipmsteven commented Feb 25, 2018

Thanks! @gauravtiwari

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

Successfully merging this pull request may close these issues.

None yet

3 participants