Skip to content
This repository was archived by the owner on Aug 6, 2025. It is now read-only.

Conversation

@jrolfs
Copy link
Member

@jrolfs jrolfs commented Apr 13, 2022

Alright, here's my take on an API that returns Locator instead of ElementHandle. It builds off of the following prior art, thanks to @sebinsua, @petetnt, and (once again) @Zache:

There's still some stuff to figure out here:

  • Scoped queries (implementing within() and ensuring that is at least as powerful as selector combinators >>)
  • Handling waiting for elements (an alternative to the findBy* queries as they don't really work as part of the selector engine which expects synchronous selection as far as I can tell) will be separate PR
  • Configuration via the configure() API ✨ Support the configure API for locator queries #450

The general idea is that, while exposing a selector engine directly is powerful, I'd like to adhere to the Testing Library API as much as possible as well as continue to delegate element selection to Testing Library queries. I do think we need to be careful here as Playwright Locators have their own way of enforcing single/multiple element selection and I'd like to keep any interaction as idiomatic as possible from the perspective of a Testing Library user.

Let me know what y'all think so far and whether this would be useful going in this direction. It seems like you probably have your own interim solutions like the linked Gist or potentially something built off of the experimental role support Playwright is adding.

@jrolfs jrolfs self-assigned this Apr 13, 2022
@jrolfs jrolfs force-pushed the feature/locator-fixtures branch from f82a499 to 6b734e9 Compare April 13, 2022 19:17
@jrolfs jrolfs force-pushed the feature/locator-fixtures branch 2 times, most recently from 2573bf0 to d0e234b Compare April 13, 2022 19:43
npm run validate
npm run test
- name: Check types, run lint + tests
Copy link
Member Author

Choose a reason for hiding this comment

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

The old version of Playwright we still test against (1.12) does not have the Locator APIs. I'll add a comment here to explain that until we drop it.

@jrolfs jrolfs force-pushed the feature/locator-fixtures branch from d0e234b to 2684221 Compare April 13, 2022 19:58
(rest, query) => ({
...rest,
[query]: (...args: Parameters<Queries[keyof Queries]>) =>
page.locator(`__testing_library__=${JSON.stringify({query, args}, replacer)}`),
Copy link
Collaborator

@sebinsua sebinsua Apr 19, 2022

Choose a reason for hiding this comment

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

Hm. I guess the loss here is that this will be a bit more difficult to read in the Playwright Inspector, however there are pros in that it gives us full usage of the Testing Library API and we're unlikely to ever clash with the Playwright Locator API (I've already done this with the recent addition of a new role= selector API into Playwright as I'd used the same name.)

I still kind of like the idea of more meaningful locators, like, by-role="roleName" {optionalArg1: "", optionalArg2: ""} but maybe it doesn't add that much?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm. I guess the loss here is that this will be a bit more difficult to read in the Playwright Inspector

This is a great point. I should be testing this out with the inspector for sure. I guess my intention here was to avoid conflicts (🤔 pretty unlikely) and make it very clear that we're serializing stuff and that the selectors are not meant to be used directly. I think you're right, though. I'll play around with the inspector.

Copy link
Member Author

Choose a reason for hiding this comment

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

})
})

// TODO: scoped queries with `within`
Copy link
Collaborator

@sebinsua sebinsua Apr 19, 2022

Choose a reason for hiding this comment

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

How are you thinking this API will work? Will it take Locators and return the queries interface? Does within come through the fixture?

const messagesLocator = screen.getByText('messages') const helloMessageLocator = within(messagesLocator).getByText('hello') expect(await helloMessageLocator.count()).toEqual(1)

I'm unsure how different the locator combinator Playwright approach is from how within normally works...!

Copy link
Member Author

Choose a reason for hiding this comment

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

The documentation mentions a create method, which I was thinking we might be able to use to scope queries but it doesn't show an implementation in the example. I'm hoping we can just leverage the combinator here.

@jrolfs
Copy link
Member Author

jrolfs commented Apr 23, 2022

Thanks for the review @sebinsua! — I'll follow up with some changes and finish up within etc. when I get a chance.

@jrolfs jrolfs force-pushed the feature/locator-fixtures branch from 2684221 to f0b77a5 Compare April 26, 2022 23:44
@jrolfs
Copy link
Member Author

jrolfs commented Apr 27, 2022

@sebinsua made some changes per your feedback, let me know what you think

better-selector-names

better-selector-names--regex

I'm not sure it's worth trying to take this any further now that things are more readable. It's pretty convenient to be able to serialize args via JSON.stringify JSON.parse.

@jrolfs jrolfs force-pushed the feature/locator-fixtures branch from 000ded2 to 3ca72c6 Compare May 2, 2022 18:58
jrolfs added 4 commits May 2, 2022 12:00
…ries This will likely replace the fixtures that provided `ElementHandle`-based queries in a future major release, but for now the `Locator` queries are exported as `locatorFixtures`: ```ts import { test as baseTest } from '@playwright/test' import { locatorFixtures as fixtures, LocatorFixtures as TestingLibraryFixtures, within } from '@playwright-testing-library/test/fixture'; const test = baseTest.extend<TestingLibraryFixtures>(fixtures); const {expect} = test; test('my form', async ({queries: {getByTestId}}) => { // Queries now return `Locator` const formLocator = getByTestId('my-form'); // Locator-based `within` support const {getByLabelText} = within(formLocator); const emailInputLocator = getByLabelText('Email'); // Interact via `Locator` API 🥳 await emailInputLocator.fill('email@playwright.dev'); // Assert via `Locator` APIs 🎉 await expect(emailInputLocator).toHaveValue('email@playwright.dev'); }) ```
Copy link
Collaborator

@sebinsua sebinsua left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I noticed:

 // TODO: configuration // TDOO: deferred page (do we need some alternative to `findBy*`?)

So maybe you still have some changes that you want to do.

@jrolfs
Copy link
Member Author

jrolfs commented Jun 1, 2022

For anyone looking to test drive this while I sort the last two bits up there, see this comment: #430 (comment)

@jrolfs
Copy link
Member Author

jrolfs commented Aug 26, 2022

This is in beta now, I'll finish the findBy* stuff in a separate PR.

@jrolfs jrolfs closed this Aug 26, 2022
@jrolfs jrolfs deleted the feature/locator-fixtures branch August 26, 2022 20:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

3 participants