Closed bigopon closed 5 years ago
For readability/maintainability this is nothing but improvement, no question there. Some of this should have happened a long time ago, but it's good to see it happening now :) I have a vague gut feeling about the switch to async/await that timings are going to be affected. It makes it feel like a somewhat risky PR. But the long term benefit can be really big. Looking forward to those integration tests 😆
Big thanks to @bigopon and @davismj for breathing a lot of new life like this into the router over the last several months. This looks like a nice set of improvements (in addition to the new capability). I want to make sure that @davismj reviews this thoroughly and that you are aligned on whether to use the viewModel
name for the property. I think Matt has proposed a different name for use in the vNext API and I'm wondering if we should try to align the names or if it doesn't matter too much (since router is probably going to be the biggest felt breaking change in vNext).
@davismj I've rebased and updated dependency route recognizer. It's working good
Came here from https://discourse.aurelia.io/t/discussion-bundler-friendly-aurelia/959/17. Any news on this?
@tmueller This work has been merged. It's now waiting for some checks and reviews, and I'm working with @davismj here https://github.com/aurelia/templating-router/pull/84 to complete this feature.
Closing this as it has been merged in another PR. Thanks to @tmueller for reminding
Thanks @bigopon for the fast response :)
Changes:
moduleId
of previous instruction and next view port config of the same view port name. This was simple when it is only eitherstring
ornull
. And 2 different view model factories could actually point to the same view model class, it needs to be resolved in advance. To do this without destroying readability, I converted the_buildNavigationPlan
to async function. This will lead to slightly bigger build, but we can start distributinges2018
so folks can use nativeasync/await
. And I took the opportunity to convert a lot of promises toasync/await
for better overall readability too.NavigationInstruction.prototype.addViewPortInstruction
, overload it to accept a view port name and a partial view port config instead of 4 primitive paramters. This makes it easier to pass view model around and keep the API clean. (Should we also deprecate/remove the old API?)viewModel
property on theconfig
,templating-router
Router needs to be updated to account for this. I will add a link to specific lines that address this in near futureTODO:
@davismj @EisenbergEffect