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

Validate homeserver configuration prior to loading the app #9779

Merged
merged 22 commits into from
May 23, 2019

Conversation

turt2live
Copy link
Member

See matrix-org/matrix-react-sdk#3001

There are no new commits on this branch. The kind of review for this PR is really just a formal green light to merge this whole thing - the individual parts have already been merged. This PR also serves as a changelog entry for when we release.

Includes:

See react-sdk PR at the top of this post for list of fixed issues.

Implements the process described here: #9290 (comment)

The expectation is that later layers (like the react-sdk) will make use of the `validated_discovery_config` option instead of interpreting the config themselves.

We intentionally block the UI from loading here to avoid races between discovery and the app loading.
It's usable as-is, and we can add things to it when we need to (ie: integrations).
Supply a server config to the component and adjust the wait logic to be less of a race. The Login component will noop onPasswordLogin if it is "busy", and it is busy when it requests the login flows.
Validate default homeserver config before loading the app
We don't actually need to do anything because the app transparently handles this.

See #9290
For use in the rest of the app.

See #9290
This doesn't cover default_server_name because that pulls in a questionable amount of JS.

See #9290
Clarify comment on is_url and hs_url handling
Flag the validated config as the default config
Show resolved homeserver configuration on the mobile guide
@turt2live turt2live marked this pull request as ready for review May 21, 2019 02:17
@turt2live turt2live requested a review from a team May 21, 2019 02:17
@jryans jryans requested review from jryans and removed request for a team May 21, 2019 08:30
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good! 😁 I would suggest waiting until after the RC on Wednesday before merging.

src/vector/index.js Show resolved Hide resolved
let config = await getVectorConfig('..');

// We manually parse the config similar to how validateServerConfig works because
// calling that function pulls in roughly 4mb of JS we don't use.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One day we'll actually fix our build to make sense... 😭

test/app-tests/loading.js Show resolved Hide resolved
@turt2live
Copy link
Member Author

RC cut, let's land this thing 🎉

@turt2live turt2live merged commit afdaca0 into develop May 23, 2019
@turt2live turt2live deleted the travis/feature/wellknown2 branch May 23, 2019 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants