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

Add an option to configure the MutationObserver #4286

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

imliam
Copy link

@imliam imliam commented Jul 4, 2024

Problem:

We are using Alpine on a website with a large DOM and many nodes changing from third-party scripts that we don't have control over. We've found that Alpine's MutationObserver causes problems when watching the entire document, notably:

  1. An increase in INP and CPU usage, as Alpine's mutation callback triggers a lot
  2. Race conditions with other scripts modifying the DOM

Solution:

The option added in this PR lets us call Alpine.start(false) with the false flag, causing the MutationObserver only to watch [x-data] elements instead of the entire document.

Of course, this means some functionality will not work as intended when a mutation happens outside an Alpine component. Still, for a page where each Alpine component only cares about its contents, this has a huge performance benefit with no real downsides.

I'm not sure if an option in Alpine.start() is preferable of there should be some other way to configure this, but the option seems to work for our use case.

Liam Hammett and others added 2 commits July 4, 2024 15:27
…server to only observe Alpine components instead of the entire document
@ekwoka
Copy link
Contributor

ekwoka commented Jul 5, 2024

We talked about this a bit on Discord, but I'll repeat/elaborate here for the discussion.

As Rich Harris recently said

Configuration is cowardly — it removes the burden of designing something correctly off maintainers' shoulders and puts it on users instead

So is there a solution to this issue that doesn't involve a configuration option?

The goal of the mutation observer is to

  • Watch for elements and attributes in components to be added and removed to initialize and cleanup handlers

It also then watches the whole document (and not just known components) so as to

  • Initialize new top level x-data added to the page

The performance issues mainly come from the fact that every element added/moved/removed is processed as if it is an alpine component, even when it isn't. Currently, this also actually allows for a bug where alpine attributes that require a context can be on element added to the page outside of a component, and they'll be initialized improperly.

So I think there is room to tackle the performance issues, while maintaining both the above behaviors.

Mainly two observers:

  1. Watches for all attribute/element changes inside a component
  2. Watches for new top level x-data (and x-init) outside components

Having these two separate can reduce how often the handlers are invoked, and reduce the overhead of processing completely non-alpine mutation entries.

How much exactly this would improve performance, I'm not totally sure, but I know initializing a tree where none exists is definitely far too costly.

I believe currently, x-ignore will prevent the subtree from being initialized when the walker gets to it, but will not stop the mutation observer from watching inside of it. I might even go so far as to say that's a bug. That elements added within an x-ignore should also be ignored by the mutation observer. This would allow some declarative user land tuning of the observers scope.

@calebporzio
Copy link
Collaborator

Yeah, I think I agree with these points @ekwoka. Ideally we find a way to not unnecessarily tax mutations. I'd be interested in a deeper discussion on this with some benchmarks and what not. Maybe something to plan on tackling for Alpine 4

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.

3 participants