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: TOC observer race condition when disconnecting. #4012

Merged
merged 1 commit into from
Jul 22, 2023

Conversation

KevinMulhern
Copy link
Member

Because:

  • With turbo caching enabled we need to tear down the lessons table of contents(toc) before the page is cached. We were doing this by unobserving each toc item. However, in some cases this wasn't finishing fast enough and was trying to unobserve toc items after they'd already been destroyed - resulting in an exception being thrown.

This commit:

  • Create the toc intersection observer on connect and assign it as a controller attribute.
  • Disconnect the entire observer instead of unobserving each item.

Because:
* With turbo caching enabled we need to tear down the lessons table of contents(toc) before the page is cached. We were doing this by unobserving each toc item. However, in some cases this wasn't finishing fast enough and was trying to unobserve toc items after they'd already been destroyed - resulting in an exception being thrown.

This commit:
* Create the toc intersection observer on connect and assign it as a controller attribute.
* Disconnect the entire observer instead of unobserving each item.
@KevinMulhern KevinMulhern self-assigned this Jul 21, 2023
@KevinMulhern KevinMulhern added the Type: Bug Involves something that isn't working as intended label Jul 21, 2023
@KevinMulhern KevinMulhern temporarily deployed to odin-review-app-pr-4012 July 21, 2023 20:09 Inactive
this.lessonContentTarget.querySelectorAll('section[id]').forEach((section) => {
this.tocItemObserver().observe(section);
this.observer.observe(section);
Copy link
Member

Choose a reason for hiding this comment

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

👀

@KevinMulhern KevinMulhern merged commit 4089976 into main Jul 22, 2023
2 checks passed
@KevinMulhern KevinMulhern deleted the fix/toc-race-condition-errors branch July 22, 2023 12:41
Mclilzee pushed a commit to Mclilzee/theodinproject that referenced this pull request Aug 2, 2023
…4012)

Because:
* With turbo caching enabled we need to tear down the lessons table of
contents(toc) before the page is cached. We were doing this by
unobserving each toc item. However, in some cases this wasn't finishing
fast enough and was trying to unobserve toc items after they'd already
been destroyed - resulting in an exception being thrown.

This commit:
* Create the toc intersection observer on connect and assign it as a
controller attribute.
* Disconnect the entire observer instead of unobserving each item.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Involves something that isn't working as intended
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants