- Notifications
You must be signed in to change notification settings - Fork 9
✨ Separate fixtures for locator-based queries #403
Conversation
f82a499 to 6b734e9 Compare 2573bf0 to d0e234b Compare | npm run validate | ||
| npm run test | ||
| - name: Check types, run lint + tests |
There was a problem hiding this comment.
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.
d0e234b to 2684221 Compare lib/fixture/locator.ts Outdated
| (rest, query) => ({ | ||
| ...rest, | ||
| [query]: (...args: Parameters<Queries[keyof Queries]>) => | ||
| page.locator(`__testing_library__=${JSON.stringify({query, args}, replacer)}`), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #403 (comment)
test/fixture/locators.test.ts Outdated
| }) | ||
| }) | ||
| | ||
| // TODO: scoped queries with `within` |
There was a problem hiding this comment.
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...!
There was a problem hiding this comment.
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.
| Thanks for the review @sebinsua! — I'll follow up with some changes and finish up |
2684221 to f0b77a5 Compare | @sebinsua made some changes per your feedback, let me know what you think 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 |
000ded2 to 3ca72c6 Compare …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'); }) ```
sebinsua left a comment
There was a problem hiding this 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.
3ca72c6 to 19c9ebd Compare | For anyone looking to test drive this while I sort the last two bits up there, see this comment: #430 (comment) |
| This is in beta now, I'll finish the |


Alright, here's my take on an API that returns
Locatorinstead ofElementHandle. 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:
within()and ensuring that is at least as powerful as selector combinators>>)Handling waiting for elements (an alternative to thewill be separate PRfindBy*queries as they don't really work as part of the selector engine which expects synchronous selection as far as I can tell)configure()API ✨ Support the configure API for locator queries #450The 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
rolesupport Playwright is adding.