-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
fix: correct router logic #2342
Conversation
Open in CodeSandbox Web Editor | VS Code | VS Code Insiders | Preview |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
affine-toeverythingBundle maindiff ------------------- Bundle Size Diff -------------------------
@@ EntryPoint: main @@
## master …5/0511-router +/- ##
===================================================================
< Bundle 3.11 MB 3.18 MB +65.7 kB(+2.11%)
< Initial JS 1.86 MB 1.88 MB +19.2 kB(+1.03%)
< Initial CSS 16.8 kB 33.9 kB +17.2 kB(+102.45%)
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~#
< Assets 41 48 +7
< Chunks 39 42 +3
< Packages 132 138 +6
= Duplicates 3 3
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Warnings ~~~~~~~~~~~~~~~~~~~~~~~~~~~#
! Separate mixed content assets files
! Avoid cache wasting
@@ EntryPoint: pages/_app @@
## master …5/0511-router +/- ##
===================================================================
< Bundle 3 MB 3.05 MB +56.3 kB(+1.88%)
< Initial JS 1.75 MB 1.76 MB +9.8 kB(+0.56%)
< Initial CSS 16.8 kB 33.9 kB +17.2 kB(+102.45%)
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~#
< Assets 39 46 +7
< Chunks 37 40 +3
< Packages 132 138 +6
= Duplicates 3 3
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Warnings ~~~~~~~~~~~~~~~~~~~~~~~~~~~#
! Separate mixed content assets files
! Avoid cache wasting
@@ EntryPoint: pages/_error @@
## master …5/0511-router +/- ##
===================================================================
= Bundle 16.6 kB 16.6 kB
= Initial JS 16.6 kB 16.6 kB
= Initial CSS 0 B 0 B
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~#
= Assets 2 2
= Chunks 2 2
= Packages 5 5
= Duplicates 0 0
...and more |
Deploying with Cloudflare Pages
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2342 +/- ##
==========================================
+ Coverage 72.60% 72.61% +0.01%
==========================================
Files 333 328 -5
Lines 7252 7182 -70
Branches 1145 1126 -19
==========================================
- Hits 5265 5215 -50
+ Misses 1778 1766 -12
+ Partials 209 201 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
@@ -0,0 +1,139 @@ | |||
/** |
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.
I moved this test into a new file because I found that next-router-mock has some corner case which might not isolate with other test case
rootCurrentPageIdAtom.onMount = set => { | ||
if (typeof window !== 'undefined') { | ||
const callback = (url: string) => { | ||
const value = url.split('/')[3]; |
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.
hard code here, but it's ok because we don't have other use cases
useEffect(() => { | ||
const id = setTimeout(() => { | ||
if (!exist) { | ||
void push('/'); |
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.
This avoids entering a nonexisting workspace. we will fallback to the index page
@@ -152,7 +152,7 @@ export function Modals() { | |||
setOpenCreateWorkspaceModal(false); | |||
setOpenWorkspacesModal(false); | |||
setCurrentWorkspace(id); | |||
return jumpToSubPath(id, WorkspaceSubPath.SETTING); | |||
return jumpToSubPath(id, WorkspaceSubPath.ALL); |
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.
I think this is a requirement for creating new workspace that we will redirect to the settings pages after creation.
Now, we only listen to the
routeChangeStart
and don't case other things.