Support `waitFor` without a DOM #3

nickserv · 2022-12-30T09:33:10Z

Describe the feature you'd like:

I was chatting with @JacobMGEvans, who was asking if waitFor could be used to test a CLI written in Node asyncronously. I thought you should be able to waitFor any callback and options that don't depend on the DOM., but ended up getting an error with this simple example:

import assert from "assert"
import { waitFor } from "@testing-library/dom"

let done = false
setTimeout(() => (done = true), 1000)
await waitFor(() => assert(done))

/Users/nick/Downloads/waitFor/node_modules/@testing-library/dom/dist/helpers.js:32
    throw new Error('Could not find default container');
          ^

Error: Could not find default container
    at getDocument (/Users/nick/Downloads/waitFor/node_modules/@testing-library/dom/dist/helpers.js:32:11)
    at waitFor (/Users/nick/Downloads/waitFor/node_modules/@testing-library/dom/dist/wait-for.js:15:40)
    at /Users/nick/Downloads/waitFor/node_modules/@testing-library/dom/dist/wait-for.js:167:54
    at Object.asyncWrapper (/Users/nick/Downloads/waitFor/node_modules/@testing-library/dom/dist/config.js:23:23)
    at waitForWrapper (/Users/nick/Downloads/waitFor/node_modules/@testing-library/dom/dist/wait-for.js:167:35)
    at file:///Users/nick/Downloads/waitFor/test.js:6:7
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

@testing-library/dom version: 8.19.1

Suggested implementation:

Run conditional checks on anything that requires a DOM implementing. However, I'm not against assuming there is a DOM when the user attempts to pass DOM elements explicitly.

Ideally this should also work without Jest or Node, as it's valid to run CLI tests in any JavaScript runtime or testing framework, and the user likely wouldn't need Jest to conveniently set up a DOM and our matchers.

Describe alternatives you've considered:

We could publish a separate package that doesn't depend on the DOM, but I personally don't think it's worth the maintenance burden for a single function. Alternatively, we could make our own testing library with additional utilities for CLIs, though I'd want us to reach consensus on overall CLI testing goals before implementing new APIs.

Teachability, Documentation, Adoption, Migration Strategy:

A simple patch to the waitFor function wouldn't require any documentation, though a separate package would.

The text was updated successfully, but these errors were encountered:

eps1lon · 2022-12-30T11:06:00Z

Yeah I've heard this a couple of times. But it should be a separate package. It doesn't make sense to have waitFor in /dom not depend on the DOM. In the end, /dom would just use Promise.race([waitForWithMutationObserver(), waitForRuntimeAgnostic()]) where waitForRuntimeAgnostic is what you're asking for.

Though we can't be fully runtime agnostic since stuff like setTimeout is platform specific. So maybe something that runs in runtimes implementing the Minimum Common Web Platform API i.e. maybe @testing-library/web (though that wouldbe awkard to use with React Native)?

JacobMGEvans · 2022-12-30T14:18:30Z

Yeah I've heard this a couple of times. But it should be a separate package. It doesn't make sense to have waitFor in /dom not depend on the DOM. In the end, /dom would just use Promise.race([waitForWithMutationObserver(), waitForRuntimeAgnostic()]) where waitForRuntimeAgnostic is what you're asking for.

Though we can't be fully runtime agnostic since stuff like setTimeout is platform specific. So maybe something that runs in runtimes implementing the Minimum Common Web Platform API i.e. maybe @testing-library/web (though that wouldbe awkard to use with React Native)?

I also suggested last night it be it's own package. Additionally, the agnostic part would be test-runners, it would still be running on Node (potentially Deno I guess if you wanted). The reason being I plan on moving from Jest to Vitest and that was brought up as a compatibility issue for Testing Library being somewhat tied to Jest.

eps1lon · 2022-12-30T14:30:12Z

We're only tied to Jest with regards to fake timer support in which we don't use MutationObserver anyway. So I don't really understand why this is a Jest vs Vitest issue unless Vitest uses another DOM implementation that's worse than JSDOM with regards to implementation progress.

If there's an issue with Vitest, please file a separate issue. This should "just" work (assuming Vitest has setup the DOM environment properly).

JacobMGEvans · 2022-12-30T14:48:51Z

We're only tied to Jest with regards to fake timer support in which we don't use MutationObserver anyway. So I don't really understand why this is a Jest vs Vitest issue unless Vitest uses another DOM implementation that's worse than JSDOM with regards to implementation progress.

If there's an issue with Vitest, please file a separate issue. This should "just" work (assuming Vitest has setup the DOM environment properly).

I was clarifying that it wasn't a runtime issue, that the "agnostic" aspect was initially around making sure it can run on other test runners, AVA, Vitest, etc... and should run on Node without the DOM as the initial issue indicated. The use case is tests in the CLoudflare Wrangler CLI where I am doing a lot of stuff manually that could just be waitFor which I was discussing in my Twitter Space and Nick thought would be an interesting.

eps1lon · 2022-12-30T15:38:20Z

I was clarifying that it wasn't a runtime issue

I want to be very clear to set expectations: This issue is about making sure waitFor works without a DOM implementation. Test runners are not related to this issue since waitFor already works with any test runner that implements a DOM (which Vitest and AVA can do). If not, then this would be considered a bug and needs a repro.

The only thing that's not supported is fake timers other than the ones provided by the global jest object.

Vitest, AVA etc are not what we usually consider a "runtime" (apart from the globals these test runners may inject). Node.js, Deno, Cloudflare workers etc are what is usually considered the "runtime".

If you want fake timer support of other test runners, we have testing-library/dom-testing-library#987 instead

JacobMGEvans · 2022-12-30T16:08:34Z

I am almost certain we are saying the same thing @eps1lon

The only thing that's not supported is fake timers other than the ones provided by the global jest object.

Likely the "slightly tied to Jest" thing myself and Nick had talked about last night. I had said "agnostic part would be test-runners"

Vitest, AVA etc are not what we usually consider a "runtime" (apart from the globals these test runners may inject). Node.js, Deno, Cloudflare workers etc are what is usually considered the "runtime".

I don't think anyone here was making a claim otherwise, pretty sure all of us here know what a "runtime" is. Again in my case in which I discussed with Nick "agnostic part would be test-runners" (because it moving from Jest to Vitest) "it would still be running on Node" (The Cloudflare Workers CLI runs on Node)

nickserv · 2022-12-30T17:39:00Z

Yeah I've heard this a couple of times. But it should be a separate package.

Why? Would this break anything? We already have conditional feature flags for other things, I think this would be a pretty minor fix compared to trying to release and maintain a new package.

Though we can't be fully runtime agnostic since stuff like setTimeout is platform specific.

In theory, maybe. But in practice, I'm not aware of a JavaScript runtime used for testing that doesn't support setTimeout.

So maybe something that runs in runtimes implementing the Minimum Common Web Platform API i.e. maybe @testing-library/web (though that wouldbe awkard to use with React Native)?

I don't think the name web would be appropriate considering it's valid to implement this spec without supporting other web APIs.

Parity with native is a good point, though. I think it could be helpful to have a base implementation of waitFor that doesn't depend on the DOM or React Native, then DOM/RN Testing Library could use it internally. Can we reach consensus on that? Then where we put that implementation and how we expose it would be an implementation detail.

If there's an issue with Vitest, please file a separate issue. This should "just" work (assuming Vitest has setup the DOM environment properly).

This issue isn't specific to Vitest, which is my reproduction doesn't use it. The way I see it, you should be able to use an API that doesn't require a DOM without a DOM. While Vitest does support multiple DOM implementations which could cause compatibility issues with us, it's also valid to use a different test framework, or Jest without a DOM (which is actually the default config now).

@eps1lon Please do not edit my labels without my permission unless you can prove that they're incorrect. I have provided a valid reproduction. If you still don't think this is a bug, then please provide a reproduction of a passing test using waitFor without a DOM.

timdeschryver · 2023-01-02T17:24:17Z

As @eps1lon mentioned, I also think that this would be better as a separate package.

The reasoning is that the package name @testing-library/dom mentions DOM, so I do find the current behavior OK. If we would add it then as I user I would find it weird to install @testing-library/dom to write tests for a CLI or Node environment.

To be blunt, maybe creating a separate repo would be an overkill for just a simple method that waits for something?
Even if this introduces a @testing-library/{web,node,cli}, I wouldn't assume that we make use of it in this library.

In the past, we've had a request to support a CLI - if this is something that gains more traction we can add it to the Testing Library family IF it will be maintained.

Links:

nickserv · 2023-01-02T19:28:31Z

If this package is only for the DOM, then why do some of its tests depend on a Node environment? I'd argue it is already environment agnostic enough that users maybe consider parts of it need to work without a DOM, considering this is open source and they could read our tests.

alexkrolick · 2023-01-06T22:57:16Z

As I recall, it originally was a separate package that got vendored in and customized with the MutationObserver stuff.

https://github.com/TheBrainFamily/wait-for-expect/blob/master/src/index.ts

eps1lon · 2023-02-09T12:17:46Z

Initial implementation proposal: #2. Comments welcome.

Will merge #2 soon-ish as an alpha, use in dom-testing-library in a MINOR and, once all initial bugs are ironed out, release @testing-library/web as stable from which you can use waitFor that supports all web-interoperable runtimes

nickserv added bug Something isn't working enhancement New feature or request labels Dec 30, 2022

eps1lon removed the bug Something isn't working label Dec 30, 2022

nickserv added the bug Something isn't working label Dec 30, 2022

JacobMGEvans mentioned this issue Jan 12, 2023

🚀 Feature Request: Await waitUntils in unstable_dev cloudflare/workers-sdk#2474

Closed

eps1lon self-assigned this Jan 14, 2023

eps1lon linked a pull request Jan 14, 2023 that will close this issue

feat: Implement waitFor #2

Open

eps1lon transferred this issue from testing-library/dom-testing-library Feb 9, 2023

eps1lon removed the bug Something isn't working label Feb 9, 2023

AriPerkkio mentioned this issue Sep 2, 2023

Built-in waitFor and waitUntil vitest-dev/vitest#4061

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `waitFor` without a DOM #3

Support `waitFor` without a DOM #3

nickserv commented Dec 30, 2022 •

edited

Loading

eps1lon commented Dec 30, 2022 •

edited

Loading

JacobMGEvans commented Dec 30, 2022

eps1lon commented Dec 30, 2022

JacobMGEvans commented Dec 30, 2022

eps1lon commented Dec 30, 2022

JacobMGEvans commented Dec 30, 2022 •

edited

Loading

nickserv commented Dec 30, 2022 •

edited

Loading

timdeschryver commented Jan 2, 2023 •

edited

Loading

nickserv commented Jan 2, 2023 •

edited

Loading

alexkrolick commented Jan 6, 2023

eps1lon commented Feb 9, 2023

Support waitFor without a DOM #3

Support waitFor without a DOM #3

Comments

nickserv commented Dec 30, 2022 • edited Loading

Describe the feature you'd like:

Suggested implementation:

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Migration Strategy:

eps1lon commented Dec 30, 2022 • edited Loading

JacobMGEvans commented Dec 30, 2022

eps1lon commented Dec 30, 2022

JacobMGEvans commented Dec 30, 2022

eps1lon commented Dec 30, 2022

JacobMGEvans commented Dec 30, 2022 • edited Loading

nickserv commented Dec 30, 2022 • edited Loading

timdeschryver commented Jan 2, 2023 • edited Loading

nickserv commented Jan 2, 2023 • edited Loading

alexkrolick commented Jan 6, 2023

eps1lon commented Feb 9, 2023

Support `waitFor` without a DOM #3

Support `waitFor` without a DOM #3

nickserv commented Dec 30, 2022 •

edited

Loading

eps1lon commented Dec 30, 2022 •

edited

Loading

JacobMGEvans commented Dec 30, 2022 •

edited

Loading

nickserv commented Dec 30, 2022 •

edited

Loading

timdeschryver commented Jan 2, 2023 •

edited

Loading

nickserv commented Jan 2, 2023 •

edited

Loading