Technical requirements for behavioural unit tests

[Iku-turso]

Implement a lot of technical requirements for behavioural unit tests

With this:

1. Routes and parameters related are strongly typed
2. Routes exist in all environments making usage transparent
3. Routes and related phenomena comply to Open Closed Principle (eg. what it means to be a new route does not require modification of existing code, but new code)
4. Behavioural unit tests are possible (see Behavioural unit tests for navigation and reference #5085)

Note: the crux of this was to make routing env-agnostic, and not based on URLs as magic strings, but instead something type-enforced.

Note: extension-based routes comply to same exact interface by "late-registering" their routes when installed. Routes are injectables which implement a shared injection token.

Note: another chunk of global shared state is no more.

Note: a lot of stuff has become reactive as part if this.

Note: a lot of explicit side effects have been cornered to injectables with causesSideEffects: true.

Note: old unit tests which want to test what is causesSideEffects: true can now do so with di.permitSideEffects(someInjectable);.

[Nokel81]

I would like to raise notice against using this. I find that even with types pipeline is very hard to read. I would like to motion that chaining is better ergonomically.

[Iku-turso]

Opinion noted! :)

Given a bit of time and familiarity, the pattern of pipeline will once again present itself as very easy to read, instead of anything else. This will become more apparent in more complex algorithms.

As with most design patterns, truly realized ergonomy comes from repetition ;)

Noteworthy: pipeline has multiple advantages over the proposed Collection chaining -pattern:

1. Better support for functional composition, and other functional patterns, such as flow or ternary functions.
2. Better refactorability, especially for nested or recursed scenarios.
3. Power to control flow and aspect: eg. if required, pipeline can abstract promises and even generators.
4. Full control of vocabulary (and not just map, reduce, filter, etc.) leads to more expressive code.
5. It can be used for other than collections (yet, so can eg. lodash.chain, but then again this).
6. Etc.

We used Collection chaining for years before growing out of it. As with everything, we do not use something like pipeline to flex, but instead for the necessity of the Greater good ;)

[panuhorsmalahti]

I think the pipeline syntax is good, but I'm a bit wary of adding this particular dependency, which appears to be quite a thin wrapper over lodash. This dependency has really small usage. Perhaps there's another alternative that is more commonly used in the ecosystem.

[Iku-turso]

@panuhorsmalahti hmm, interesting point. Here's some thoughts:

1. We control this library ourselves, and for that, it is flexible and without magic.

2. We created the library only recently, and therefore there is yet little adoption in the ecosystem.

3. We could have implemented this function in Lens as well, but it is pretty general to be local. Eg., also injectable uses it.

4. To look for an alternative full foreign library for this "thin wrapper" is a bit of an overkill, as the proposed pattern is universal, and does not require certain library to work properly with. Instead, any function with arity of one can participate in composition. Therefore a full alternate library would add extra weight with all the other bells and whistles. I mean, we already have lodash.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[jim-docker]

still going...

[jim-docker]

why do we need this? Can't navigateToAppPreferencesInjectable be used directly?

[jansav]

If you want to navigate to first page of preferences you should use navigateToPreferences. If you want to navigate to specific tab in preferences, you should use e.g. navigateToAppPreferences.

Motivation is to allow change of mind in future without need for multiple modifications.

[jim-docker]

You mean so one day navigateToAppPreferences may take optional param(s) to select the tab?

[jansav]

No. navigateToAppPreferences doesn't need any parameters. However we might change our mind that navigateToPreferences doesn't go to app preferences. One day it might go to editor preferences and for that change we would need to change lot of file instead of one.

Implementation for navigateToPreferences could even be "go to first tab in preferences" and it shouldn't need to know which tab is first. In that case navigateToPreferences doesn't have to change at all if someday we change order of tabs in preferences.

[Nokel81]

Will do more reviewing today.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[Nokel81]

My qualms about some specific design aspects should be all non-breaking to fix later.