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

Work in env without window.location #110

Merged
merged 1 commit into from
Dec 24, 2016

Conversation

pesterhazy
Copy link
Contributor

Environments like React Native don't have a window.location. In such an
environment, boot-reload fails because window.location.hostname or
.protocol does not exist. Use some-> to avoid these exceptions.

Environments like React Native don't have a window.location. In such an
environment, boot-reload fails because window.location.hostname or
.protocol does not exist. Use some-> to avoid these exceptions.
@martinklepsch
Copy link
Contributor

martinklepsch commented Dec 24, 2016 via email

@pesterhazy
Copy link
Contributor Author

pesterhazy commented Dec 24, 2016 via email

@Deraen
Copy link
Contributor

Deraen commented Dec 24, 2016

Probably this doesn't break anything.

But does this fix anything either? with React-native or other environments?

The reload mechanism currently only loads the files by appending script tags to head, and I know this won't work with Webworkers or Node.

@pesterhazy
Copy link
Contributor Author

@Deraen, yes! We've been using boot-reload very happily in boot-react-native for a year or more.

We have code that patches Google Closure's mechanism with a custom eval-based js loader:

https://github.com/mjmeintjes/boot-react-native/blob/master/resources/mattsum/boot_rn/js/reloading.js

The context of this PR: We currently shim window.location (https://github.com/mjmeintjes/boot-react-native/blob/99f656e86740b13695fc1ff3d86cdbcadfdede62/resources/mattsum/boot_rn/js/goog_base.js#L49) for boot-reload, which works but is a bit hacky. I'm trying to clean up BRN, hence the PR.

@Deraen Deraen merged commit 7daab91 into adzerk-oss:master Dec 24, 2016
@Deraen
Copy link
Contributor

Deraen commented Dec 24, 2016

Okay, makes sense.

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.

3 participants