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

Support deep linking in the dev server without hash location strategy #886

Open
natebosch opened this issue Jan 21, 2018 · 12 comments
Open
Labels
P3 A lower priority bug or feature request package:build_runner type-enhancement A request for a change that isn't a bug

Comments

@natebosch
Copy link
Member

I think this is usually done by searching upwards for an index.html instead of serving a 404.

Hopefully we can make this an option with our handler. Otherwise we should document how to get your own handler to do this.

@jakemac53
Copy link
Contributor

jakemac53 commented Jan 22, 2018

Personally I don't think this is something we should bake in, we should just provide the capabilities to add it yourself. That provides more value in the long term and will also help us have fewer breaking changes.

Other packages should be able to wrap up most of the logic, so for instance angular_router should be able to expose a handler to deal with its way of doing routing, and it should be trivial for users to add that in. Or possibly we could even allow packages to inject handlers into the dev server automagically through build.yaml config.

@matanlurey
Copy link
Contributor

Otherwise we should document how to get your own handler to do this.

This is probably the easiest route for for now.

@natebosch
Copy link
Member Author

It would be awesome if this were modular, configurable with build.yaml, and something that the Angular package could have a hand it - I wonder how quickly we can do that and how many people will be forced onto the manual path in the meantime.

I think it would be relatively straightforward to implement behind a command line flag in the short term - if we can turn around quickly with it, is there any reason we shouldn't?

@jakemac53
Copy link
Contributor

@natebosch I don't want to get stuck with it forever and don't want to support all the custom options people ultimately would want for it. I think we could run almost as quickly on a really simple solution that allows you to inject handlers?

I also think people would be super happy with a completely manual process, as long as they had some option which they don't have today.

@matanlurey
Copy link
Contributor

Maybe we just need a way to write a "serve.dart", i.e. let build_runner take care of almost everything, but override the server handler?

@natebosch
Copy link
Member Author

#888 would be one way to make it easier to let build_runner take care of the reading the config for angular/build_web_compilers but then letting the user handle the rest - including doing custom stuff with the handler.

@jakemac53
Copy link
Contributor

jakemac53 commented Jan 22, 2018

Yes, we have talked offline a bit about this in the past but one of the simplest options would be to enable users to create handlers in a dart file and either have those be imported by the main script and added to the cascade or possibly just allow you to return a completely custom cascade or server.

This could be done either by convention tool/serve.dart or config (in build.yaml). @natebosch we would likely want the config approach eventually which would make splitting things into different files harder as it would likely be a top level field (#885).

@matanlurey matanlurey added type-enhancement A request for a change that isn't a bug package:build_runner labels Jan 22, 2018
@chalin
Copy link
Contributor

chalin commented Jan 22, 2018

(This would be great for running the AngularDart examples.)

@jakemac53 jakemac53 added the P3 A lower priority bug or feature request label Sep 18, 2018
@rich-j
Copy link

rich-j commented Dec 24, 2018

Any work on this? We love pub run build_runner serve --live-reload except that it forces us to use Angular hash routing. This requires us to change code for our production releases. We also just encountered a bug with Firebase email link authentication not correctly encoding the hash (firebase/firebaseui-web#537).

Here's a quick hack to support a minimal deep-linking that we put together. In https://github.com/dart-lang/build/blob/master/build_runner/lib/src/server/server.dart around line 321 we added a few lines of code to notFound processing to return our top-level index.html:

      case UnreadableReason.notFound:
        if (fallbackToDirectoryList) {
          return shelf.Response.notFound(await _findDirectoryList(assetId));
        }
        // Hack to support Angular deep-linking
        final pathSegments = assetId.pathSegments;
        if (!pathSegments.contains("package") && !pathSegments.contains("lib")) {
          print("Angular deep link overriding ${assetId.path}");
          return _handle(requestHeaders, AssetId(_rootPackage, 'web/index.html'));
        }
        return shelf.Response.notFound('Not Found');
      default:

Of course proper support would include command line option processing and/or other implementations as discussed above. However, we were pleasantly surprised by how the Dart build system allowed us to use the above hack - download this repository, make the above changes, then in our main application's pubspec.yaml we specified the build_runner dependency as path: ../build/build_runner. When we build and serve our main app, it uses the above hack. The live reload even recognizes changes to the server code and rebuilds - cool.

@jakemac53
Copy link
Contributor

Any work on this?

Nothing yet - and I don't think we have any plans scheduled to properly fix it. We would however be receptive to pull requests, but it would be a good idea to have us review your general design before writing any code to reduce churn if you decide to go that route.

I still think the best solution would be a general mechanism for adding custom handlers, but we would be open to alternate proposals including but not limited to a custom flag that enables behavior similar to what you have above (I wouldn't be in favor of that as the default behavior though). It would probably need to be customizable as far as which page is served, and maybe only handle directory paths?

However, we were pleasantly surprised by how the Dart build system allowed us to use the above hack - download this repository, make the above changes, then in our main application's pubspec.yaml we specified the build_runner dependency as path: ../build/build_runner

That is one of the primary reasons this project doesn't live in the SDK :) , it definitely makes it easier to add custom hacks around things. Note that you will introduce a bit of extra startup time here - pub disables snapshotting of path dependencies executables, but that is probably an acceptable price to pay.

@evanweible-wf
Copy link
Contributor

Should this be moved to webdev given that the serve command is planned for removal here? (#2227)

Has any more discussion been had around doing this via build.yaml config versus a convention like tool/serve.dart? I'd be willing to take a stab at it (I was working on upgrading the proxy server we use internally, but I think this would be easier than having to proxy the SSE endpoint)

@jodinathan
Copy link

we've created a PR that add an option to redirect not found to a specific asset: #3361

it should fix this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 A lower priority bug or feature request package:build_runner type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants