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

Add client-side authentication features #8806

Merged
merged 18 commits into from Aug 7, 2020
Merged

Add client-side authentication features #8806

merged 18 commits into from Aug 7, 2020

Conversation

haijian-vaadin
Copy link
Contributor

@haijian-vaadin haijian-vaadin commented Aug 3, 2020

No description provided.

flow-client/src/test/frontend/ConnectTests.ts Outdated Show resolved Hide resolved
flow-client/src/test/frontend/ConnectTests.ts Outdated Show resolved Hide resolved
flow-client/src/test/frontend/ConnectTests.ts Outdated Show resolved Hide resolved
@haijian-vaadin haijian-vaadin changed the title WIP: Add client-side authentication features Add client-side authentication features Aug 6, 2020
@haijian-vaadin haijian-vaadin added the hilla Issues related to Hilla label Aug 6, 2020
@haijian-vaadin haijian-vaadin marked this pull request as ready for review August 6, 2020 14:07
@vaadin-bot vaadin-bot added +0.0.1 and removed +0.1.0 labels Aug 6, 2020
Copy link
Contributor

@Haprog Haprog left a comment

Choose a reason for hiding this comment

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

So now that Middleware works as a bare function or class I get it that we need to call it as middleware.call(middleware, context, next), but imo it makes the code a bit confusing as in the class case the call method gets an unnecessary parameter that's not needed.

I think it would be cleaner to have a Middleware type which would be a union of two different types, where one is like the old function type and another is the new class interface and we can detect it to call them differently. This way anyone who creates a middleware class doesn't need to wonder why there's an extra parameter needed for the call method.

Otherwise this looks good except some typos I found.

@haijian-vaadin haijian-vaadin merged commit 5083324 into master Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hilla Issues related to Hilla +0.0.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants