-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Changed How is local is checked and how serverURL is build to prevent… #1122
Conversation
… errros in offline mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not having the window.location.hostname === ''
check breaks the offline detection for local files, such as from my app :(
You mean for compiled files? Because this change works fine when run via "npm start dev". Even if you serve dist folder it should work without problems |
This is indeed back to working. Also yeah, my app is serving compiled files and registers as an empty string for that check for whatever reason; I don't know the details behind it, but that is what happens for it. My Android apk registers normally as localhost though. |
I readded the check for you |
Yep, that's what I was talking about; it works fine for my app's particular offline now. Now it's just up to the actual devs to see what they think of this change for their purposes heh |
As there were minor changes to the code, I tested this again and can confirm it fixes #1078 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, this works perfectly fine for both the dev server and my app.
This PR is ready to be reviewed/merged by a dev. |
Walker says this is fine. See https://discord.com/channels/1125469663833370665/1243948971110694982 |
src/utils.ts
Outdated
export const isLocal = ( | ||
(window.location.hostname === "localhost" || | ||
/^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$/.test(window.location.hostname)) && | ||
window.location.port !== "" | ||
) || window.location.hostname === ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs some better formatting, it looks pretty bad. Please use any kind of consistency with where you're splitting up the line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair i wouldnt split the line at all....
But then it is pretty long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a different kind of bad
Now it is consistent and breaks after || or && if it would be to long without the break |
… errros in offline mode
Fixes #1078