-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: public private routing #1474
Merged
Merged
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
This PR contains workaround for SvelteKit issue sveltejs/kit#7415 |
mstrasinskis
approved these changes
Nov 3, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks! Looking forward seeing it in main.
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.
Motivation
Instead of an up-front sign-in screen, we want user to be to sign-in on each screen. Doing so we will be able to display meaningful static information on each pages and maybe be even able to display public information while not being signed - i.e. render proposal even if not signed in.
Breaking changes
This PR switch the custom routing implementation to a SvelteKit base routing. This is a BREAKING CHANGES.
We switch from an hasbang base routing system to a query params system. In addition, we migrate from a SPA to a multi page app.
There is not backwards compatibility for the urls. Hashbang params will be ignored.
However, to ease the launch on mainnet without the need to synchronize the proposal execution with the dashboard, the PR contains a workaround to route
/#/proposal/{id}
from login page to proposal detail page. Workaround we aim to ultimately, rather soon than later, delete.Approach
We use +layout.ts file to extract the parameters (universe and details like "&proposal=123") request.
The "universe" param being a global information, it gets derived in a custom
pageStore
- i.e. we use SvelteKit $app/stores to derive our own page information.Other fine grained params - e.g &proposal=123" - a passed down from the
+page
to its content.Limitation
This PR does not support unknown routes. i.e. if user would enter
nns.ic0.app/yolo
it will face a 500 error uncatched. See "Open questions".Performance
Optimizing the performance of the dapp is a battle between loading everything in a single JS bundle files and finding the right amount of chunks.
This PR does the following:
it uses Rollup
manualchunks
to bundle allnode_modules
- except Svelte related components and code - dependencies in avendor.js
file and adapp
file for the appSvelteKit chunks the rest of the app
sometimes we might want to limit the number of chunks for fine tuning. That's why the
login
page does not use a+layout.svelte
file but includes the layout as a component or why the route are still lazy loaded with ourRouteModule
Security
The CSP parser has been extended to parse the rules in each and every single
.html
file that is generated.TODO
Following are the lists of things I am aware of currently and still need to be done.
Not in this PR
trailingSlash
strategy, we can maybe copy the files. e.g.accounts/index.html
-> clone toaccounts.html
as well). Note: This is also not supported at the moment in IIerror.html
page that we accept to make not certified?