Skip to content

Make autoloading changes visible to the DOM#57

Closed
seanpdoyle wants to merge 2 commits into
hotwired:mainfrom
seanpdoyle:autoloader-attributes
Closed

Make autoloading changes visible to the DOM#57
seanpdoyle wants to merge 2 commits into
hotwired:mainfrom
seanpdoyle:autoloader-attributes

Conversation

@seanpdoyle
Copy link
Copy Markdown
Contributor

Closes #35
Related to #49

Extend the autoloader to communicate autoloading state via
a DOMTokenList stored in the [data-stimulus-autoloading]
attribute.

The goal is to communicate state to the DOM so that tools like Capybara
can monitor the presence or absence of the attribute, and synchronize
page interactions based on whether autoloading is in-flight. By waiting,
our System Tests can act with confidence that any lazily-loaded behavior
is ready.

function extractControllerNamesFrom(element) {
return element.getAttribute(controllerAttribute).split(/\s+/).filter(content => content.length)
const tokenList = createTokenList(element, controllerAttribute)
return Array.from(tokenList).map(name => ({ element, name }))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe storing the currently autoloader identifiers on the html, body, or a <meta> element would suffice, instead of on each individual element.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What are your thoughts around placing it on html, body or meta? I was thinking that with a data-stimulus-autoloading attribute on the element directly you could at least target that with a css class and have a path to display a loading indicator that way (or block interaction).

end
end

def click_button(*)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In its current state, this draft PR only extends the click_button method, but if we find this approach viable, we'd want to extend all methods in Capybara::DSL.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a super interesting way to address this problem! One thing that I'm wrestling with is that I can't help but feel like this behavior codifies the possible experience for a user where:

  1. they click on a button, it doesn't work (controller hasn't loaded yet)
  2. they wait for a second, click on it again, and it does work (after the controller has loaded)

Granted, in most cases, the delay here should be negligible and shouldn't be a problem, but I do think there are situations where this will come up outside of an automated test scenario.

Just an idea here, but I wonder if it'd be possible to specify strategies for how to deal with this in the library that would help avoid the need to override this behavior in capybara, e.g. data-controller-autoload="hide" or data-controller-autoload="spinner" - in the former case, you could just wait for the element to become visible to click on it (which capybara already does pretty well), and in the latter, you could handle that with a helper method somewhere. By making it an explicit attribute, I think it could help illustrate to devs that this node may render before the javascript behavior has been attached to it, and you may need to deal with that.


def click_button(*)
begin
assert_css "[data-stimulus-autoloading]"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This assertion is intended to wait for the autoloading to start.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It might be best to wrap this in a using_wait_time block of 0-ish to cut down on time spent waiting.

rescue
# does not eagerly load any Stimulus
end
assert_no_css "[data-stimulus-autoloading]"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This assertion is intended to wait for the autoloading to complete.

If we think using assert calls would make the test suite assertion count fluctuate too much, there might be other ways to wait on the presence or absence of elements matching the selector.

Closes [hotwired#35][]
Related to [hotwired#49][]

Extend the autoloader to communicate autoloading state via
a [DOMTokenList][] stored in the `[data-stimulus-autoloading]`
attribute.

The goal is to communicate state to the DOM so that tools like Capybara
can monitor the presence or absence of the attribute, and synchronize
page interactions based on whether autoloading is in-flight. By waiting,
our System Tests can act with confidence that any lazily-loaded behavior
is ready.

[hotwired#35]: hotwired#35
[hotwired#49]: https://github.com/hotwired/stimulus-rails/pull/49/files#r610601520
[DOMTokenList]: https://developer.mozilla.org/en-US/docs/Web/API/DOMTokenList
@seanpdoyle seanpdoyle force-pushed the autoloader-attributes branch from 4550222 to 422acd0 Compare April 9, 2021 14:08
Addresses failure in CI:

```
Fetching gem metadata from https://rubygems.org/............
Your bundle is locked to mimemagic (0.3.5), but that version could not be found
in any of the sources listed in your Gemfile. If you haven't changed sources,
that means the author of mimemagic (0.3.5) has removed it. You'll need to update
your bundle to a version other than mimemagic (0.3.5) that hasn't been removed
in order to install.
```
@seanpdoyle seanpdoyle closed this Apr 30, 2021
@filipemendespi
Copy link
Copy Markdown

hey @seanpdoyle can you find an alternative way of the problem? I saw that @dhh didn't like the idea very much, but I was waiting for their solution to pass.

@aesnyder
Copy link
Copy Markdown

Hey @seanpdoyle I'm curious if there's a suggested work-around for this in the meantime or the best place to follow for updates? We're needing to ensure that all connects() are finished before capybara tries to interact with the page.

@agrberg
Copy link
Copy Markdown

agrberg commented May 17, 2026

I encountered a similar problem and reported the issue to rubycdp/ferrum You can see the workaround in rubycdp/ferrum#584 and a fix in rubycdp/ferrum#586

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Autoloading race conditions

5 participants