core: refactor routing #3222
Merged
core: refactor routing #3222
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Jenkins please test this please. |
saghul
force-pushed
the
refactor-router
branch
2 times, most recently
from
5a94681
to
872682e
Compare
|
Jenkins please test this please. |
|
@lyubomir This is now ready for review. Tests have passed, but somehow doesn't show it here. See: http://ci.jitsi.org/view/pr-testing/job/jitsi-meet-PR-test/4604/ |
saghul
force-pushed
the
refactor-router
branch
2 times, most recently
from
1647006
to
37ca3dd
Compare
Unfortunately, as the Jitsi Meet development evolved the routing mechanism became more complex and thre logic ended up spread across multiple parts of the codebase, which made it hard to follow and extend. This change aims to fix that by rewriting the routing logic and centralizing it in (pretty much) a single place, with no implicit inter-dependencies. In order to arrive there, however, some extra changes were needed, which were not caught early enough and are thus part of this change: - JitsiMeetJS initialization is now synchronous: there is nothing async about it, and the only async requirement (Temasys support) was lifted. See [0]. - WebRTC support can be detected early: building on top of the above, WebRTC support can now be detected immediately, so take advantage of this to simplify how we handle unsupported browsers. See [0]. The new router takes decissions based on the Redux state at the time of invocation. A route can be represented by either a component or a URl reference, with the latter taking precedence. On mobile, obviously, there is no concept of URL reference so routing is based solely on components. [0]: jitsi/lib-jitsi-meet#779
lyubomir
force-pushed
the
refactor-router
branch
from
37ca3dd
to
c203215
Compare
lyubomir
approved these changes
Jul 12, 2018
saghul
added a commit
to saghul/jitsi-meet
that referenced
this pull request
Aug 14, 2018
Your truly refactored routing in jitsi#3222 and broke it. When a bare room is entered the pathname was not updated when applying the default URL.
saghul
added a commit
to saghul/jitsi-meet
that referenced
this pull request
Aug 14, 2018
Yours truly refactored routing in jitsi#3222 and broke it. When a bare room is entered the pathname was not updated when applying the default URL.
saghul
added a commit
that referenced
this pull request
Aug 16, 2018
Yours truly refactored routing in #3222 and broke it. When a bare room is entered the pathname was not updated when applying the default URL.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unfortunately, as the Jitsi Meet development evolved the routing mechanism
became more complex and thre logic ended up spread across multiple parts of the
codebase, which made it hard to follow and extend.
This change aims to fix that by rewriting the routing logic and centralizing it
in (pretty much) a single place, with no implicit inter-dependencies.
In order to arrive there, however, some extra changes were needed, which were
not caught early enough and are thus part of this change:
it, and the only async requirement (Temasys support) was lifted. See 0.
support can now be detected immediately, so take advantage of this to simplify
how we handle unsupported browsers. See 0.
The new router takes decissions based on the Redux state at the time of
invocation. A route can be represented by either a component or a URl reference,
with the latter taking precedence. On mobile, obviously, there is no concept of
URL reference so routing is based solely on components.