-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Don't rely on module-level state for Playwright selector engine regis… #1027
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
Don't rely on module-level state for Playwright selector engine regis… #1027
Conversation
…tration (#1022) # why This is to fix some undesired behavior for a common dev workflow with `next dev`. Introduced in #954. There is now module-level state in `lib/StagehandPage.ts` (the `stagehandSelectorRegistered` boolean) used to guard against multiple calls to `selectors.register` (a Playwright function which sets module-level state). This used in the function `ensureStagehandSelectorEngine`. This guard exists because calling `selectors.register` with the same string more than once will cause an error. The problem is that `next dev` repeatedly reloads the `stagehand` module whenever we first start up our dev server or make changes, but without always reloading the underlying `playwright` module. <details> <summary>So, we get lots of errors like this.</summary> ```zsh [2025-08-22 12:58:10] web:dev: Error in Inngest task { [2025-08-22 12:58:10] web:dev: error: { [2025-08-22 12:58:10] web:dev: error: 'NonRetriableError', [2025-08-22 12:58:10] web:dev: message: "Hey! We're sorry you ran into an error. \n" + [2025-08-22 12:58:10] web:dev: 'Stagehand version: 2.4.3 \n' + [2025-08-22 12:58:10] web:dev: 'If you need help, please open a Github issue or reach out to us on Slack: https://stagehand.dev/slack\n' + [2025-08-22 12:58:10] web:dev: '\n' + [2025-08-22 12:58:10] web:dev: 'Full error:\n' + [2025-08-22 12:58:10] web:dev: 'selectors.register: "stagehand" selector engine has been already registered', [2025-08-22 12:58:10] web:dev: name: 'Error', [2025-08-22 12:58:10] web:dev: stack: 'StagehandDefaultError: \n' + [2025-08-22 12:58:10] web:dev: "Hey! We're sorry you ran into an error. \n" + [2025-08-22 12:58:10] web:dev: 'Stagehand version: 2.4.3 \n' + [2025-08-22 12:58:10] web:dev: 'If you need help, please open a Github issue or reach out to us on Slack: https://stagehand.dev/slack\n' + [2025-08-22 12:58:10] web:dev: '\n' + [2025-08-22 12:58:10] web:dev: 'Full error:\n' + [2025-08-22 12:58:10] web:dev: 'selectors.register: "stagehand" selector engine has been already registered\n' + [2025-08-22 12:58:10] web:dev: ' at _StagehandPage.eval (webpack-internal:///(rsc)/../../node_modules/.pnpm/@browserbasehq+stagehand@2.4.3_deepmerge@4.3.1_dotenv@16.6.1_react@18.3.1_zod@3.25.67/node_modules/@browserbasehq/stagehand/dist/index.js:4077:15)\n' + [2025-08-22 12:58:10] web:dev: ' at Generator.throw (<anonymous>)\n' + [2025-08-22 12:58:10] web:dev: ' at rejected (webpack-internal:///(rsc)/../../node_modules/.pnpm/@browserbasehq+stagehand@2.4.3_deepmerge@4.3.1_dotenv@16.6.1_react@18.3.1_zod@3.25.67/node_modules/@browserbasehq/stagehand/dist/index.js:73:29)' [2025-08-22 12:58:10] web:dev: }, ``` </details> **TL;DR The `stagehand` module state guard, to guard the Playwright module state, becomes out of sync with Playwright.** This is not really `stagehand`'s "fault". It appears to be `next`-specific behavior combined with some logic to get around funky module-level `playwright` state. But it is causing a lot of friction on our team; I think module-level state is risky in general for this reason. # what changed My proposed fix is to wrap this `selectors.`register call in a specific `try`/`catch` that looks for, and ignores, the specific error `/selector engine has been already registered/` in `packages/playwright-core/src/client/selectors.ts` instead of using the `stagehandSelectorRegistered` boolean. # test plan Existing evals. And this works locally as expected when I build our system against this version, but without the error, no matter how many times the module is reloaded.
🦋 Changeset detectedLatest commit: c8dfac0 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Greptile Summary
This PR refactors the Playwright selector engine registration mechanism in StagehandPage.ts
by removing dependency on module-level state tracking. The change replaces a global boolean flag (stagehandSelectorRegistered
) with a try-catch approach that handles Playwright's "selector engine has been already registered" error gracefully.
Previously, the code relied on a process-level boolean variable to prevent duplicate registrations of the 'stagehand' selector engine. This approach had potential issues in environments with multiple library instances, hot reloading, or testing scenarios where module state might not be reliable. The new implementation attempts to register the selector engine on every call and catches the specific error that Playwright throws when attempting to register an already-registered engine, simply ignoring this expected error.
The refactor also improves code organization by extracting the registration function into a separate variable (registerFn
) before passing it to selectors.register()
. This change aligns with common library patterns for handling idempotent operations and makes the code more resilient across different runtime environments.
Confidence score: 5/5
- This PR is safe to merge with minimal risk
- Score reflects a simple, well-understood refactoring that eliminates potential state management issues without changing core functionality
- No files require special attention
1 file reviewed, no comments
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @browserbasehq/stagehand@2.4.4 ### Patch Changes - [#1012](#1012) [`9e8c173`](9e8c173) Thanks [@miguelg719](https://github.com/miguelg719)! - Fix disabling api validation whenever a customLLM client is provided ## @browserbasehq/stagehand-evals@1.0.8 ### Patch Changes - Updated dependencies \[[`9e8c173`](9e8c173)]: - @browserbasehq/stagehand@2.4.4 ## @browserbasehq/stagehand-examples@1.0.8 ### Patch Changes - Updated dependencies \[[`9e8c173`](9e8c173)]: - @browserbasehq/stagehand@2.4.4 ## @browserbasehq/stagehand-lib@2.4.1 ### Patch Changes - [#1027](#1027) [`455b61f`](455b61f) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - Fixed small issue with module-level state guard for the Playwright selectors.register call Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
detailed description here