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

Jump to bottom

Autoupdate client does not honor WebAppInternals.setBundledJsCssPrefix #7149

Closed
egoldblum opened this issue Jun 1, 2016 · 5 comments
Closed

Autoupdate client does not honor WebAppInternals.setBundledJsCssPrefix #7149

egoldblum opened this issue Jun 1, 2016 · 5 comments
Assignees
@tmeasday
Labels
confirmed We want to fix or implement it Impact:some Severity:production Type:Bug
Milestone
Release 1.3.4.2

Comments

@egoldblum
Copy link

Currently running 1.2.1 on Galaxy.

We set WebAppInternals.setBundledJsCssPrefix within a Meteor.startup. In the general case this works fine.

Observed behavior:
After deploying new code, connected browser clients reload automatically, but requests for the new js/css land on our origin not the CDN.

Expected behavior:
Autoupdate'd css link href (and javscript script src) should continue to respect the set WebAppInternals.setBundledJsCssPrefix

https://github.com/meteor/meteor/blob/devel/packages/autoupdate/autoupdate_client.js#L131
@laosb
Copy link
Contributor

laosb commented Jun 4, 2016

Yeah, should be and did reproduce on a self-hosted app.

@tmeasday tmeasday self-assigned this Jun 28, 2016
@tmeasday
Copy link
Contributor

A simple way to reproduce this locally:

# in one terminal
meteor create testapp
cd testapp
echo "WebAppInternals.setBundledJsCssPrefix('http://localhost:4000')" >> server/main.js
meteor

# in another
git clone https://github.com/tmeasday/simple-proxy
cd simple-proxy
npm install
node index.js

@tmeasday
Copy link
Contributor

@egoldblum I am only seeing the behavior you are referring to for CSS files, not JS files. Are you sure JS files don't hit the CDN?

tmeasday added a commit that referenced this issue Jun 28, 2016
@tmeasday
Publish CSS documents for autoupdate after rewriting for #7149
f2280ba 
Previously we did not take the JsCssRewriteHook (i.e. CDN host)
into account when doing CSS-only "version-refreshable" HCP.

It is not possible on the client to rewrite CSS urls to take the
hook into account (as it is a function), so instead, we just
publish those CSS URLs post-transform.
@tmeasday tmeasday mentioned this issue Jun 28, 2016
Merged
@egoldblum
Copy link
Author

@tmeasday you are correct, only rewritten css links cause requests to our origin, not js.

tmeasday added a commit that referenced this issue Jun 29, 2016
@tmeasday
Publish CSS documents for autoupdate after rewriting for #7149
4be08cb 
Previously we did not take the JsCssRewriteHook (i.e. CDN host)
into account when doing CSS-only "version-refreshable" HCP.

It is not possible on the client to rewrite CSS urls to take the
hook into account (as it is a function), so instead, we just
publish those CSS URLs post-transform.
benjamn added a commit that referenced this issue Jun 29, 2016
@benjamn
Merge pull request #7316 from meteor/7149-CSS-respect-CDN-settings
f19241c 
Publish CSS documents for autoupdate after rewriting for #7149.
@benjamn benjamn added this to the Release 1.3.4.2 milestone Jul 7, 2016
@abernix abernix added the confirmed We want to fix or implement it label Jul 16, 2016
@abernix
Copy link
Contributor

abernix commented Jul 16, 2016

This is confirmed fixed in Meteor 1.3.4.2! Thanks for reporting this problem, @egoldblum.

@abernix abernix closed this as completed Jul 16, 2016
@tmeasday tmeasday mentioned this issue Aug 9, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tmeasday tmeasday

Labels
confirmed We want to fix or implement it Impact:some Severity:production Type:Bug
Projects
None yet
Milestone
Release 1.3.4.2
Development

No branches or pull requests
5 participants
@benjamn @egoldblum @tmeasday @abernix @laosb