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

Don't error when running hosted.html require in dist/ vs. src/ #732

Closed
wants to merge 1 commit into from

Conversation

humphd
Copy link

@humphd humphd commented Apr 19, 2017

People constantly complain about the requirejs error we have when we load in dist/. This doesn't rely on require to fail before trying to load in dist. NOTE: this only affects hosted.html (so testing).

@@ -131,12 +133,15 @@
}

// Support loading from src/ or dist/ to make local dev easier
require(["bramble/client/main"], function(Bramble) {
if(window.location.pathname === "/dist/hosted.html") {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't work on staging/production because of S3 object prefixes.
You would probably need to regex this instead (or to be safer, have a meta tag in the html indicating that it is a hosted version and query that)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll rework.

Speaking of this, can you tell me how we do paths for staging/prod so I can also fix my URL scheme for cached URLS in #549?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's <domain>/bramble/<staging|production>/dist/index.html
where <domain> is mozillathimblelivepreview.net and it's either staging or production in the path.

@gideonthomas gideonthomas assigned humphd and unassigned gideonthomas Apr 21, 2017
@humphd
Copy link
Author

humphd commented Jun 8, 2017

I have a better fix for this in #782.

@humphd humphd closed this Jun 8, 2017
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.

2 participants