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

Handle file protocol on initialisation #38

Closed
wants to merge 1 commit into from
Closed

Handle file protocol on initialisation #38

wants to merge 1 commit into from

Conversation

jtfell
Copy link

@jtfell jtfell commented Jul 30, 2017

When trying to bundle my found-relay (modern) react app into an electron distributable, I realised that the routing wouldn't work on first load as the pathname would be something like /home/jtfell/project/dist/index.html when I really wanted to start at /.

This change seems to have fixed that, with all subsequent internal links working as relative paths. I'm not at all sure if this is the best approach to handle all the edge-cases surrounding the file protocol but it works for me.

One issue I ran into is that making the change here causes it not to be triggered when the page is refreshed. Do you have any ideas on a better place to slot this in @taion? Even if you don't want to merge this it would be great to get your thoughts.

@taion
Copy link
Contributor

taion commented Jul 30, 2017

I don't think this is quite right. I think you either want to use base name, or just use an in-memory protocol or a hash protocol. The former would be mostly trivial to implement, while the latter would take a bit more work.

The issue here is that, while this might work for Electron, it wouldn't be quite right outside of Electron; there's no way you could refresh the page and end up back where you want to be.

@jquense
Copy link
Member

jquense commented Jul 30, 2017

We should think about open sourcing/including the in memory protocol I wrote for our Electron app btw

@taion
Copy link
Contributor

taion commented Jul 30, 2017

Huh, I had no idea that existed.

@jquense
Copy link
Member

jquense commented Jul 31, 2017

Yeah I meant to send it over afterwards but moved on to other stuff and forgot about it :P

@taion
Copy link
Contributor

taion commented Dec 22, 2017

@jtfell Sorry for the delay here. We're recommending going with #44 for Electron use cases.

@taion taion closed this Dec 22, 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.

3 participants