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

Fixed discrepancies in page store between server and client #119

Merged
merged 4 commits into from
Nov 17, 2020

Conversation

ehrencrona
Copy link
Contributor

@ehrencrona ehrencrona commented Nov 3, 2020

Host was null on the server side using dev server. The query was a URLSearchParams on the server and a map on the client.

(This should be safeguarded by types; I'll add that once my other PRs regarding types are merged)

Fixes #118 (except for error being null vs undefined; that's fixed by #108)

});
}

search.slice(1).split('&').filter(Boolean).forEach(searchParam => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there was a teeny tiny bug in here: the query string ?a=b& would become translated to {a: 'b', '': ''}

as for the rest of the function i refactored it because i think this is more readable and also shorter

@benmccann
Copy link
Member

Looks like this one has a merge conflict

@ehrencrona ehrencrona force-pushed the same-page-context-client-and-server branch from eb13ad8 to f700856 Compare November 9, 2020 08:07
@ehrencrona
Copy link
Contributor Author

rebased

@benmccann
Copy link
Member

This should be safeguarded by types; I'll add that once my other PRs regarding types are merged

You can probably do that now that a couple of the other PRs have been checked in

@ehrencrona ehrencrona force-pushed the same-page-context-client-and-server branch from f700856 to 4676fcb Compare November 17, 2020 08:52
@ehrencrona
Copy link
Contributor Author

I moved PageContext to app-utils to make it possible to get it type safe.

Would be good to get this merged now; there will quickly be new conflicts in the types.

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
@ehrencrona ehrencrona merged commit 5cf5d3e into master Nov 17, 2020
@ehrencrona ehrencrona deleted the same-page-context-client-and-server branch November 17, 2020 09:03
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.

Server-side page store automatic context association does not work
3 participants