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

Use CDN for chunks as well #1084

Closed
TheRusskiy opened this issue Dec 12, 2017 · 18 comments
Closed

Use CDN for chunks as well #1084

TheRusskiy opened this issue Dec 12, 2017 · 18 comments

Comments

@TheRusskiy
Copy link
Contributor

Right now asset_host setting seems to only affect the main JS file.
The main JS pack file is loading from CDN correctly:
https://cdn.mysite.com/packs/admins-fb39652d413e925cd086.js
but then it requests a chunk and it ignores the CDN setting:
https://www.mysite.com/packs/0-d21df208925975fc5ed1.chunk.js

@TheRusskiy
Copy link
Contributor Author

Are this issue and #1080 the same ones?

@gauravtiwari
Copy link
Member

Yes, sounds similar. Lets fix it 👍

@gauravtiwari
Copy link
Member

So this one looks a bit tricky because webpack seems to trying to load the assets asynchronously and assuming the url to be the current document url. If we supply full public path into the webpack build then Rails will try to add the path with it's asset_host too, which will result in duplication.

A workaround is to include following in a file where chunks are declared:

// app/javascript/helpers/public-path.js

const removeOuterSlashes = string =>
   string.replace(/^\/*/, '').replace(/\/*$/, '')

__webpack_public_path__ = `${removeOuterSlashes(process.env.WEBPACKER_ASSET_HOST)}/packs` // whatever the public output path is
// packs/application.js

import '../helpers/public-path'
//... rest of the imports

@TheRusskiy
Copy link
Contributor Author

@gauravtiwari in order for this to work do I need to try with master branch? ( so #1107 is included)

@gauravtiwari
Copy link
Member

Yes please although it should work with 3.0.2 as well 👍

@gauravtiwari gauravtiwari removed the bug label Dec 15, 2017
@TheRusskiy
Copy link
Contributor Author

TheRusskiy commented Dec 16, 2017

@gauravtiwari I am getting a pretty weird behaviour now.
On initial page load (I have an SPA) the first chunk is loaded from the normal host, when I navigate to a different page and webpack loads another chunk it does it from the CDN.
So it's:
initial page load
https://cdn.mysite.com/packs/admins-e330b16d30c53aaf59b9.js
https://www.mysite.com/packs/0-2ae1b2669d9cdd1c9988.chunk.js
navigating to another SPA page
https://cdn.mysite.com/packs/1-1abbd151f1f6e4e9a7ba.chunk.js

p.s. also I made it ${removeOuterSlashes(process.env.WEBPACKER_ASSET_HOST)}/packs/ instead of ${removeOuterSlashes(process.env.WEBPACKER_ASSET_HOST)}/packs (extra slash in the end.
p.p.s using 3.2.0

I have

if (process.env.NODE_ENV === "production") {
  require("common/helpers/publicPath")
}

as a first line in my pack file.
Is it somehow possible that webpack is loading the first chunk before reading this line?

@gauravtiwari
Copy link
Member

@TheRusskiy Have you setup CDN for rails?

# production.rb
config.action_controller.asset_host = 'http://example.com'

@TheRusskiy
Copy link
Contributor Author

@gauravtiwari yes :-)

@gauravtiwari
Copy link
Member

Does this chunk file: https://www.mysite.com/packs/0-2ae1b2669d9cdd1c9988.chunk.js

if (process.env.NODE_ENV === "production") {
  require("common/helpers/publicPath")
}

has this import at the top? If you have multiple packs you would need to include it in all packs. Or you can make a pack just for public path and put at the top, somewhere in the <head></head>

//packs/public-path.js

if (process.env.NODE_ENV === "production") {
  require("common/helpers/publicPath")
}
<head>
<%= javascript_pack_tag 'public-path' %>
</head>

@TheRusskiy
Copy link
Contributor Author

@gauravtiwari
That chunk doesn't have it. The "main" file has it.
Tried including "public path" chunk before the other one per your suggestion - same problem.
Also tried doing window.__webpack_public_path__ = ... (added explicit window), inspected in the browser, the property is set but no effect.

@TheRusskiy
Copy link
Contributor Author

Webpack should be doing all of this for us already.
All we need to do is to set publicPath https://github.com/rails/webpacker/blob/master/package/environment.js#L71
In the end I did this in production.js

const host = "https://cdn.mysite.com"
const config = require("@rails/webpacker/package/config")
config.publicPath = `${host}/packs/`
const environment = require("./environment")
module.exports = environment.toWebpackConfig()

Which is very hacky but gets the job done.
Is there some API we can use to make this a bit prettier?

@gauravtiwari
Copy link
Member

@TheRusskiy Yes, setting like this will apply CDN at compile time and not runtime, which means you might get duplicate CDN URLs (one from Rails and one from webpack compile).

const environment = require("./environment")
const { config } = require("@rails/webpacker")

const host = "https://cdn.mysite.com" // process.env.ASSET_HOST
environment.config.publicPath = `${host}/${config.public_output_path}/`

module.exports = environment.toWebpackConfig()

@TheRusskiy
Copy link
Contributor Author

isn't

const config = require("@rails/webpacker/package/config")
config.publicPath = `${host}/packs/`

just override to

environment.config.publicPath = `${host}/${config.public_output_path}/`

?

I don't see publicPath changing in other places.

@gauravtiwari
Copy link
Member

You don't want to mutate original config object but instead environment specific webpack config.

config.publicPath = `${host}/packs/`

this mutates internal config object, not the webpack config object.

This changes production specific option for webpack:

environment.config.publicPath = `${host}/${config.public_output_path}/`

@navied
Copy link

navied commented Dec 21, 2017

Not sure if this is related by since upgrading to 3.2.0 image assets that are within the manifest.json are not being added with the proper CDN url attached with it. I can only assume it is related to #1107

Edit: Noticed this issue was posted before 3.2.0 was released might make sense to turn this into it's own issue.

@nddery
Copy link

nddery commented Jan 22, 2018

We've tried updating __webpack_public_path__ per the original comment by @gauravtiwari but it didn't had any repercussion, our dynamic imports were still not using the CDN url. What ended up working for us was to update the output.publicPath webpack option like so (on 3.0.2) :

// config/webpack/production.js
const { assetHost } require('@rails/webpacker')

if (process.env.WEBPACKER_ASSET_HOST) {
  environment.config.set('output.publicPath', assetHost.publicPathWithHost)
}

We then upgraded to 3.2.0, which removed assetHost.publicPathWithHost, and had to replace the publicPath with the following:

// config/webpack/production.js
const { config } = require('@rails/webpacker')

if (process.env.WEBPACKER_ASSET_HOST) {
  const publicPath = `${process.env.WEBPACKER_ASSET_HOST}/${config.public_output_path}/`
  environment.config.set('output.publicPath', publicPath)
}

Not sure what the exact difference between ASSET_HOST and WEBPACKER_ASSET_HOST is - seems like the former is the actual environment variable while the latter is the one from Rails (config.action_controller.asset_host).

It's also been mentioned that this would result in duplication, not sure what is meant exactly but this.

I also noticed that in 3.0.2 the file loader used to include the publicPath with publicPathWithHost, but that is not the case anymore in 3.2.0 (and at some point before the URL loader did the same I believe). We aren't using the file loaded at this point but I can only assume it'll lead to some issues, like the ones mentioned by @navied above. (edit that's the issue at #1186)

@ernsheong
Copy link

ernsheong commented Jul 27, 2018

Changing ASSET_HOST in configuration.js to WEBPACKER_ASSET_HOST seems to do it for me.

Change from:

const output = {
  path: resolve("public", settings.public_output_path),
  publicPath: formatPublicPath(env.ASSET_HOST, settings.public_output_path)
};

To:

const output = {
  path: resolve("public", settings.public_output_path),
  publicPath: formatPublicPath(env.WEBPACKER_ASSET_HOST, settings.public_output_path)
};

gauravtiwari added a commit that referenced this issue Dec 14, 2018
 New split chunks API
 New file loader option for adding static extensions
 Basic node modules compilation apart from app code
 CDN support - #1084 Perhaps someone can help with this one?
@gauravtiwari
Copy link
Member

Please try the latest RC, this is now fixed.

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

No branches or pull requests

5 participants