Support path based routing in dev-server

[antriver]

Short description of what this resolves:

Assuming you wish to use the path location strategy in the Ionic app

locationStrategy: 'path'

and are running ionic serve.
Currently you can load your index page and navigate around fine. The path in the browser is updated to /about for example. However if you refresh this /about page, you get a 404 because that path does not exist.

Changes proposed in this pull request:

Added a catchall route so that 404 will get sent to the index page and the Ionic deeplinker can handle routing to the about segment.

[lincolnthree]

Awesome! This is exactly what the doctor ordered. I second this motion. Thanks for submitting this! Hoping to see it get merged soon.

[lincolnthree]

Confirmed this patch works for me with no issues.

[FdezRomero]

WOW 😳

Adding support for path location strategy in just one line is awesome. BTW, it should be app.get(serveIndex); like the routes above.

It doesn't even need to add the slashes to the tags in injector.ts, as you need to have a base href in the index.html anyways, so it will work just fine.

@danbucholtz please consider merging/adding this one-liner.

[danbucholtz]

How does this work with the deep linker? Sorry, I have so much going on I haven't had a chance to take a look.

@FdezRomero, does this break anything? Working well for you?

Thanks,
Dan

[FdezRomero]

The only thing preventing people from using locationStrategy: 'path' in development is that when you do ionic serve, the app-scripts Express server will try to resolve navigation segments (deep links) as folders (as there's no /#/ anymore). As they don't exist as folders, requests will return 404.

Adding app.get(serveIndex); at the end rewrites all GET requests not matching any folder/route, loading index.html with a 200 status instead of 404.

It's a server-side thing, so the deep linker is not affected. This line doesn't break anything and works well. The changes in injector.ts need to be discarded from this PR because that's automatically handled by the browser with a <base href="/your_path"> tag in index.html, and people may want to use their own base paths.

Thanks,
Rodrigo

[lincolnthree]

I added this so that I could use the deep linker locally. Without this patch, your local dev server will fail to resolve /anything/ and throw 404s everywhere if you've navigated anywhere in the app besides the root component.

I'd say it works fine. I haven't run into any issues with it, but I did get tired of hacking it every time new app scripts came out, so I used environments to work around it for the time being. Would still love this so I can actually debug my apps using production-like URLs.

[danbucholtz]

@antriver, want to remove the injector changes? Sounds like we can merge the other part. I haven't tested yet but I will.

Thanks,
Dan

[subhash-ztech]

@FdezRomero
Just a clarification based on your previous comment, if the changes in injector.js of this pull request are removed, then should it be app.get(serveIndex) or app.use(serveIndex)?

[FdezRomero]

@subhash-ztech They're not related. app.get() matches GET requests only, while app.use() matches all HTTP methods (Express docs).

In this case, because we are serving index.html it doesn't make sense to serve POST, PUT, DELETE... requests to the index, that's why I suggested to use app.get() instead, like the Ionic routes. But both should work fine.

[subhash-ztech]

@FdezRomero Thanks for the clarification.

[FdezRomero]

Testing on the 2.0.0 release I noticed that @antriver was right about his injector.ts changes:
if the leading slashes are not added, ion-dev.js is redirected to the index and doesn't load the script (and errors trying to parse <). The <base href> isn't applied here.

Apologies for my mistake @danbucholtz, commit df1583e needs to be reverted and slashes to be added 🙏

[antriver]

Hi @danbucholtz ,

Could you explain what this broke? In 065912e you say it "broke proxies". Can you elaborate on that so I can fix this if necessary and we can actually get it released.

Also, as mentioned above, the injector changes are needed. Otherwise ion-dev.js and ion-dev.css don't work. Those changes (adding a / to the start of those URLs) are only being consistent with how they're defined in http-server.ts anyway (they have a leading slash there on line 37)

[danbucholtz]

@antriver,

I don't know the details but people that need to run ionic serve through a proxy were having issues. It was unable to serve the correct file or something. I honestly didn't validate it, I just saw a bunch of people reporting the issue so I quickly reverted.

Thanks,
Dan

[salbahra]

Is this something that can be added as a flag? To allow the developers who do need this functionality?

Or, are there any other solutions which I am missing regarding this issue?

[ericeslinger]

If the base href tag is put higher in the html file than the injected scripts, one does not need to prepend slashes. This is what I do - my index.html file has the base as the third thing: <html><head><base href="/"> and it works pretty okay, because I was having just this problem when working with location mode.

[himanshu-singh1995]

Although this pull request is merge but I am unable to find these changes in here in http-server.ts.

I set up an Ionic App (ionic version 3.20.0) with path location strategy. I had the same issue. I investaged and reached here and found that the changes in this pull request are missing in my node modules (in node_modules/@ionic/app-scripts/dist/dev-server/http-server.js).

[jayvdb]

It was reverted in 065912e , and there is another PR at #1339