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

fix(Suspense): correctly update the transition anchor point #9999

Merged
merged 6 commits into from
Jan 8, 2024

Conversation

baiwusanyu-c
Copy link
Member

close: #9996

Copy link

github-actions bot commented Jan 4, 2024

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 89.4 kB (+8 B) 34 kB (+5 B) 30.7 kB (+2 B)
vue.global.prod.js 146 kB (+8 B) 53.3 kB (+4 B) 47.6 kB (-21 B)

Usages

Name Size Gzip Brotli
createApp 49.7 kB 19.5 kB 17.8 kB
createSSRApp 53.1 kB 20.8 kB 19 kB
defineCustomElement 52 kB 20.2 kB 18.5 kB
overall 63.2 kB (+8 B) 24.4 kB (+5 B) 22.2 kB (-18 B)

@baiwusanyu-c baiwusanyu-c marked this pull request as draft January 4, 2024 14:20
@baiwusanyu-c baiwusanyu-c marked this pull request as ready for review January 4, 2024 14:21
@baiwusanyu-c
Copy link
Member Author

It works as expected in the playground, but the result is incorrect in e2e. I may need some guidance.

@baiwusanyu-c

This comment was marked as outdated.

@baiwusanyu-c baiwusanyu-c marked this pull request as draft January 4, 2024 16:15
@edison1105
Copy link
Member

edison1105 commented Jan 5, 2024

It works as expected in the playground, but the result is incorrect in e2e. I may need some guidance.

Did you forget to run nr build vue -f global -d before running the e2e test?

@baiwusanyu-c baiwusanyu-c marked this pull request as ready for review January 5, 2024 04:29
@baiwusanyu-c
Copy link
Member Author

baiwusanyu-c commented Jan 5, 2024

@baiwusanyu-c
Copy link
Member Author

baiwusanyu-c commented Jan 5, 2024

Did you forget to run nr build vue -f global -d before running the e2e test?

The test results of e2e on my window machine, mac, and vue ci may be inconsistent every time I run it.

@edison1105
Copy link
Member

I think we only need to obtain an anchor once and cache it. No matter how the branch is switched internally, the anchor will not change.

Like this:

if (activeBranch) {
  // if the fallback tree was mounted, it may have been moved
  // as part of a parent suspense. get the latest anchor for insertion
-  anchor = next(activeBranch)
+  anchor = suspense.__anchor !== undefined 
+		? suspense.__anchor 
+       : (suspense.__anchor = next(activeBranch))
  unmount(activeBranch, parentComponent, suspense, true)
}
          activeBranch!.transition!.afterLeave = () => {
            if (pendingId === suspense.pendingId) {
              move(
                pendingBranch!,
                container,
-				next(activeBranch!),
+                anchor,
                MoveType.ENTER,
              )
              queuePostFlushCb(effects)
            }
          }

@baiwusanyu-c
Copy link
Member Author

Good idea, already modified

@edison1105
Copy link
Member

I took a closer look, and caching the anchor may not work. If the anchor is uninstalled, there may still be problems with rendering.

😂😂😂

@baiwusanyu-c
Copy link
Member Author

So for special processing like I did before, what about just doing the cache anchor in case1 and case2?

@edison1105
Copy link
Member

edison1105 commented Jan 6, 2024

I took a deep look at #8105 and found the cause for the error is that the anchor is inside the hiddenContainer, so the anchor is not valid. We just need to get the latest anchor during afterLeave.

try

// 1
move(
  pendingBranch!,
  container,
  anchor === suspense.anchor
    ? next(activeBranch!)
    : anchor,
  MoveType.ENTER,
)

// 2
if (activeBranch) {
  // if the fallback tree was mounted, it may have been moved
  // as part of a parent suspense. get the latest anchor for insertion
  // #8105 if `delayEnter` is true, it means that the mounting of `activeBranch` will be delayed. 
  // if the branch switches before transition completes, both `activeBranch` and `pendingBranch` may coexist 
  // in the `hiddenContainer`. This could result in `next(activeBranch!)` obtaining 
  // an incorrect anchor (got `pendingBranch.el`). 
  // Therefore, after the mounting of activeBranch is completed, 
  // it is necessary to get the latest anchor.
  if(parentNode(activeBranch.el!) !== suspense.hiddenContainer){
    anchor = next(activeBranch)
  }
  unmount(activeBranch, parentComponent, suspense, true)
}

@yyx990803 yyx990803 merged commit a3fbf21 into vuejs:main Jan 8, 2024
11 checks passed
@baiwusanyu-c baiwusanyu-c deleted the bwsy/fix/9996 branch January 28, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suspense + out-in Transition renderes component out of order
3 participants