diff --git a/.changeset/chilled-mails-eat.md b/.changeset/chilled-mails-eat.md new file mode 100644 index 00000000..29c05487 --- /dev/null +++ b/.changeset/chilled-mails-eat.md @@ -0,0 +1,5 @@ +--- +'pleasantest': minor +--- + +Improve forgot-await detection diff --git a/src/async-hooks.ts b/src/async-hooks.ts new file mode 100644 index 00000000..b08939b8 --- /dev/null +++ b/src/async-hooks.ts @@ -0,0 +1,57 @@ +/** + * Manages asynchronous calls within withBrowser. + * If any async calls are "left over" when withBrowser finishes executing, + * then that means the user forgot to await the async calls, + * so we should throw an error that indicates that. + */ + +import { removeFuncFromStackTrace } from './utils'; + +/** + * Set of all active async hook trackers + * We need to store this module-level so that jest-dom matchers can know which withBrowser they "belong" to + * If there are multiple active at a time, the jest-dom matchers won't include the forgot-await behavior. + */ +export const activeAsyncHookTrackers = new Set(); + +export interface AsyncHookTracker { + addHook( + func: () => Promise, + captureFunction: (...args: any[]) => any, + ): Promise; + close(): Error | undefined; +} + +export const createAsyncHookTracker = (): AsyncHookTracker => { + const hooks = new Set(); + let isClosed = false; + + const addHook: AsyncHookTracker['addHook'] = async ( + func, + captureFunction, + ) => { + const forgotAwaitError = new Error( + 'Cannot interact with browser after test finishes. Did you forget to await?', + ); + removeFuncFromStackTrace(forgotAwaitError, captureFunction); + hooks.add(forgotAwaitError); + try { + return await func(); + } catch (error) { + if (!isClosed) throw error; + } finally { + if (!isClosed) hooks.delete(forgotAwaitError); + } + }; + + const close = () => { + if (isClosed) return; + isClosed = true; + activeAsyncHookTrackers.delete(asyncHookTracker); + if (hooks.size > 0) return hooks[Symbol.iterator]().next().value as Error; + }; + + const asyncHookTracker: AsyncHookTracker = { addHook, close }; + activeAsyncHookTrackers.add(asyncHookTracker); + return asyncHookTracker; +}; diff --git a/src/extend-expect.ts b/src/extend-expect.ts index 621618ef..76f9d40c 100644 --- a/src/extend-expect.ts +++ b/src/extend-expect.ts @@ -1,4 +1,6 @@ import type { ElementHandle, JSHandle } from 'puppeteer'; +import type { AsyncHookTracker } from './async-hooks'; +import { activeAsyncHookTrackers } from './async-hooks'; import { createClientRuntimeServer } from './module-server/client-runtime-server'; import { deserialize, serialize } from './serialize'; import { @@ -40,184 +42,169 @@ const isJSHandle = (input: unknown): input is JSHandle => { return input.asElement && input.dispose && input.evaluate; }; -expect.extend( - Object.fromEntries( - methods.map((methodName) => { - const matcher = async function ( - this: jest.MatcherUtils, - elementHandle: ElementHandle | null, - ...matcherArgs: unknown[] - ) { - const serverPromise = createClientRuntimeServer(); - if (!isElementHandle(elementHandle)) { - // Special case: expect(null).not.toBeInTheDocument() should pass - if (methodName === 'toBeInTheDocument' && this.isNot) { - // This is actually passing but since it is isNot it has to return false - return { pass: false }; - } +const matchers: jest.ExpectExtendMap = Object.fromEntries( + methods.map((methodName) => { + const matcher = async function ( + this: jest.MatcherUtils, + elementHandle: ElementHandle | null, + ...matcherArgs: unknown[] + ): Promise { + const serverPromise = createClientRuntimeServer(); + if (!isElementHandle(elementHandle)) { + // Special case: expect(null).not.toBeInTheDocument() should pass + if (methodName === 'toBeInTheDocument' && this.isNot) { + // This is actually passing but since it is isNot it has to return false + return { pass: false, message: () => '' }; + } - const message = [ - this.utils.matcherHint( - `${this.isNot ? '.not' : ''}.${methodName}`, - 'received', - '', - ), + const message = [ + this.utils.matcherHint( + `${this.isNot ? '.not' : ''}.${methodName}`, + 'received', '', - `${this.utils.RECEIVED_COLOR( - 'received', - )} value must be an HTMLElement or an SVGElement.`, - isPromise(elementHandle) - ? `Received a ${this.utils.RECEIVED_COLOR( - 'Promise', - )}. Did you forget to await?` - : this.utils.printWithType( - 'Received', - elementHandle, - this.utils.printReceived, - ), - ].join('\n'); - const error = new Error(message); - - // Manipulate the stack trace and remove this function - // That way Jest will show a code frame from the user's code, not ours - // https://kentcdodds.com/blog/improve-test-error-messages-of-your-abstractions - Error.captureStackTrace(error, matcher); - throw error; - } + ), + '', + `${this.utils.RECEIVED_COLOR( + 'received', + )} value must be an HTMLElement or an SVGElement.`, + isPromise(elementHandle) + ? `Received a ${this.utils.RECEIVED_COLOR( + 'Promise', + )}. Did you forget to await?` + : this.utils.printWithType( + 'Received', + elementHandle, + this.utils.printReceived, + ), + ].join('\n'); + throw removeFuncFromStackTrace(new Error(message), matcher); + } - for (const arg of matcherArgs) { - if ( - typeof arg === 'object' && - typeof (arg as any)?.asymmetricMatch === 'function' - ) { - const error = new Error( - `Pleasantest does not support using asymmetric matchers in browser-based matchers + for (const arg of matcherArgs) { + if ( + typeof arg === 'object' && + typeof (arg as any)?.asymmetricMatch === 'function' + ) { + const error = new Error( + `Pleasantest does not support using asymmetric matchers in browser-based matchers Received ${this.utils.printReceived(arg)}`, - ); - - // Manipulate the stack trace and remove this function - // That way Jest will show a code frame from the user's code, not ours - // https://kentcdodds.com/blog/improve-test-error-messages-of-your-abstractions - Error.captureStackTrace(error, matcher); - throw error; - } + ); + throw removeFuncFromStackTrace(error, matcher); } + } - const forgotAwait = removeFuncFromStackTrace( - new Error( - `Cannot execute assertion ${methodName} after test finishes. Did you forget to await?`, - ), - matcher, - ); - /** Handle error case for Target Closed error (forgot to await) */ - const handleExecutionAfterTestFinished = (error: any) => { - if (/target closed/i.test(error.message)) { - throw forgotAwait; - } + const { port } = await serverPromise; - throw error; - }; + const ctxString = JSON.stringify(this); // Contains stuff like isNot and promise + const result = await elementHandle.evaluateHandle( + // Using new Function to avoid babel transpiling the import + // @ts-expect-error pptr's types don't like new Function + new Function( + 'element', + '...matcherArgs', + `return import("http://localhost:${port}/@pleasantest/jest-dom") + .then(({ jestContext, deserialize, ...jestDom }) => { + const context = { ...(${ctxString}), ...jestContext } + try { + const deserialized = matcherArgs + .slice(1) + .map(a => typeof a === 'string' ? deserialize(a) : a) + return jestDom.${methodName}.call(context, element, ...deserialized) + } catch (error) { + return { thrown: true, error } + } + })`, + ), + elementHandle, + ...matcherArgs.map((arg) => (isJSHandle(arg) ? arg : serialize(arg))), + ); - const { port } = await serverPromise; + // Whether the matcher threw (this is different from the matcher failing) + // The matcher failing means that it returned a result for Jest to throw + // But a matcher throwing means that the input was invalid or something + const thrownError = await result.evaluate((result) => result.thrown); - const ctxString = JSON.stringify(this); // Contains stuff like isNot and promise - const result = await elementHandle + // We have to evaluate the message right away + // because Jest does not accept a promise from the returned message property + const message = await result.evaluate( + thrownError + ? (matcherResult) => matcherResult.error.message + : (matcherResult) => matcherResult.message(), + ); + const deserializedMessage = runJestUtilsInNode(message, this as any); + const { messageWithElementsRevived, messageWithElementsStringified } = + await elementHandle .evaluateHandle( - // Using new Function to avoid babel transpiling the import // @ts-expect-error pptr's types don't like new Function new Function( - 'element', - '...matcherArgs', + 'el', + 'message', `return import("http://localhost:${port}/@pleasantest/jest-dom") - .then(({ jestContext, deserialize, ...jestDom }) => { - const context = { ...(${ctxString}), ...jestContext } - try { - const deserialized = matcherArgs - .slice(1) - .map(a => typeof a === 'string' ? deserialize(a) : a) - return jestDom.${methodName}.call(context, element, ...deserialized) - } catch (error) { - return { thrown: true, error } - } - })`, - ), - elementHandle, - ...matcherArgs.map((arg) => - isJSHandle(arg) ? arg : serialize(arg), - ), - ) - .catch(handleExecutionAfterTestFinished); - - // Whether the matcher threw (this is different from the matcher failing) - // The matcher failing means that it returned a result for Jest to throw - // But a matcher throwing means that the input was invalid or something - const thrownError = await result.evaluate((result) => result.thrown); - - // We have to evaluate the message right away - // because Jest does not accept a promise from the returned message property - const message = await result.evaluate( - thrownError - ? (matcherResult) => matcherResult.error.message - : (matcherResult) => matcherResult.message(), - ); - const deserializedMessage = runJestUtilsInNode(message, this as any); - const { messageWithElementsRevived, messageWithElementsStringified } = - await elementHandle - .evaluateHandle( - // @ts-expect-error pptr's types don't like new Function - new Function( - 'el', - 'message', - `return import("http://localhost:${port}/@pleasantest/jest-dom") - .then(({ reviveElementsInString, printElement }) => { - const messageWithElementsRevived = reviveElementsInString(message) - const messageWithElementsStringified = messageWithElementsRevived + .then(({ reviveElementsInString, printElement }) => { + const messageWithElementsRevived = reviveElementsInString(message) + const messageWithElementsStringified = messageWithElementsRevived .map(el => { if (el instanceof Element) return printElement(el) - return el + return el }) .join('') - return { messageWithElementsRevived, messageWithElementsStringified } - })`, - ), - deserializedMessage, - ) - .then(async (returnHandle) => { - const { + return { messageWithElementsRevived, messageWithElementsStringified } + })`, + ), + deserializedMessage, + ) + .then(async (returnHandle) => { + const { + messageWithElementsRevived, + messageWithElementsStringified, + } = Object.fromEntries(await returnHandle.getProperties()); + return { + messageWithElementsStringified: + await messageWithElementsStringified.jsonValue(), + messageWithElementsRevived: await jsHandleToArray( messageWithElementsRevived, - messageWithElementsStringified, - } = Object.fromEntries(await returnHandle.getProperties()); - return { - messageWithElementsStringified: - await messageWithElementsStringified.jsonValue(), - messageWithElementsRevived: await jsHandleToArray( - messageWithElementsRevived, - ), - }; - }); - if (thrownError) { - const error = new Error(messageWithElementsStringified as any); - // @ts-expect-error messageForBrowser is a property we added to Error - error.messageForBrowser = messageWithElementsRevived; + ), + }; + }); + if (thrownError) { + const error = new Error(messageWithElementsStringified as any); + // @ts-expect-error messageForBrowser is a property we added to Error + error.messageForBrowser = messageWithElementsRevived; - // Manipulate the stack trace and remove this function - // That way Jest will show a code frame from the user's code, not ours - // https://kentcdodds.com/blog/improve-test-error-messages-of-your-abstractions - Error.captureStackTrace(error, matcher); - throw error; - } + throw removeFuncFromStackTrace(error, matcher); + } - return { - ...((await result.jsonValue()) as any), - message: () => messageWithElementsStringified, - messageForBrowser: messageWithElementsRevived, - }; + return { + ...((await result.jsonValue()) as any), + message: () => messageWithElementsStringified, + messageForBrowser: messageWithElementsRevived, }; + }; - return [methodName, matcher]; - }), - ), + const matcherWrapper = async function ( + this: jest.MatcherUtils, + elementHandle: ElementHandle | null, + ...matcherArgs: unknown[] + ): Promise { + const asyncHookTracker: AsyncHookTracker | false = + activeAsyncHookTrackers.size === 1 && + activeAsyncHookTrackers[Symbol.iterator]().next().value; + if (asyncHookTracker) { + const res = await asyncHookTracker.addHook( + () => matcher.call(this, elementHandle, ...matcherArgs), + matchers[methodName], + ); + // AddHook resolves to undefined if the function throws after the async hook tracker closes + // Because it needs to not trigger an unhandled promise rejection + if (res === undefined) return { pass: !this.isNot, message: () => '' }; + return res; + } + return matcher.call(this, elementHandle, ...matcherArgs); + }; + + return [methodName, matcherWrapper]; + }), ); const runJestUtilsInNode = (message: string, context: jest.MatcherContext) => { @@ -256,6 +243,8 @@ const runJestUtilsInNode = (message: string, context: jest.MatcherContext) => { .replace(/\\./g, (match) => JSON.parse(`"${match}"`)); }; +expect.extend(matchers); + // These type definitions are incomplete, only including methods we've tested // More can be added from https://unpkg.com/@types/testing-library__jest-dom/index.d.ts // You can copy-paste and change the return types to promises diff --git a/src/index.ts b/src/index.ts index 3e397cb1..cf74c501 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,13 +11,15 @@ import _ansiRegex from 'ansi-regex'; import { fileURLToPath } from 'url'; import type { PleasantestUser } from './user'; import { pleasantestUser } from './user'; -import { assertElementHandle, removeFuncFromStackTrace } from './utils'; +import { assertElementHandle } from './utils'; import type { ModuleServerOpts } from './module-server'; import { createModuleServer } from './module-server'; import { cleanupClientRuntimeServer } from './module-server/client-runtime-server'; import { Console } from 'console'; import { createBuildStatusTracker } from './module-server/build-status-tracker'; import { sourceMapErrorFromBrowser } from './source-map-error-from-browser'; +import type { AsyncHookTracker } from './async-hooks'; +import { createAsyncHookTracker } from './async-hooks'; export { JSHandle, ElementHandle } from 'puppeteer'; koloristOpts.enabled = true; @@ -109,12 +111,11 @@ export const withBrowser: WithBrowser = (...args: any[]) => { const testPath = testFile ? relative(process.cwd(), testFile) : thisFile; return async () => { - const { state, cleanupServer, ...ctx } = await createTab({ + const { cleanupServer, asyncHookTracker, ...ctx } = await createTab({ testPath, options, }); const cleanup = async (leaveOpen: boolean) => { - state.isTestFinished = true; if (!leaveOpen || options.headless) { await ctx.page.close(); } @@ -125,7 +126,14 @@ export const withBrowser: WithBrowser = (...args: any[]) => { try { await testFn(ctx); + const forgotAwaitError = asyncHookTracker.close(); + if (forgotAwaitError) throw forgotAwaitError; } catch (error) { + const forgotAwaitError = asyncHookTracker.close(); + if (forgotAwaitError) { + await cleanup(false); + throw forgotAwaitError; + } const messageForBrowser: undefined | unknown[] = // This is how we attach the elements to the error from testing-library error?.messageForBrowser || @@ -218,12 +226,11 @@ const createTab = async ({ options: WithBrowserOpts; }): Promise< PleasantestContext & { - state: { isTestFinished: boolean }; cleanupServer: () => Promise; + asyncHookTracker: AsyncHookTracker; } > => { - /** Used to provide helpful warnings if things execute after the test finishes (usually means they forgot to await) */ - const state = { isTestFinished: false }; + const asyncHookTracker = createAsyncHookTracker(); const browser = await connectToBrowser('chromium', headless); const browserContext = await browser.createIncognitoBrowserContext(); const page = await browserContext.newPage(); @@ -290,111 +297,89 @@ const createTab = async ({ await page.goto(`http://localhost:${port}`); - /** Runs page.evaluate but it includes forgot-await detection */ - const safeEvaluate = async ( - caller: (...params: any) => any, - ...args: Parameters - ) => { - const forgotAwaitError = removeFuncFromStackTrace( - new Error( - `Cannot interact with browser using ${caller.name} after test finishes. Did you forget to await?`, - ), - caller, - ); - return page.evaluate(...args).catch((error) => { - if (state.isTestFinished && /target closed/i.test(error.message)) { - throw forgotAwaitError; - } - - throw error; - }); - }; - - const runJS: PleasantestUtils['runJS'] = async (code, args) => { - // For some reason encodeURIComponent doesn't encode ' - const encodedCode = encodeURIComponent(code).replace(/'/g, '%27'); - const buildStatus = createBuildStatusTracker(); - // This uses the testPath as the url so that if there are relative imports - // in the inline code, the relative imports are resolved relative to the test file - const url = `http://localhost:${port}/${testPath}?inline-code=${encodedCode}&build-id=${buildStatus.buildId}`; - const res = await safeEvaluate( - runJS, - new Function( - '...args', - `return import(${JSON.stringify(url)}) - .then(async m => { - if (m.default) await m.default(...args) - }) - .catch(e => - e instanceof Error - ? { message: e.message, stack: e.stack } - : e)`, - ) as () => any, - ...(Array.isArray(args) ? (args as any) : []), - ); - - const errorsFromBuild = buildStatus.complete(); - // It only throws the first one but that is probably OK - if (errorsFromBuild) throw errorsFromBuild[0]; - - await sourceMapErrorFromBrowser(res, requestCache, port, runJS); - }; - - const injectHTML: PleasantestUtils['injectHTML'] = async (html) => { - await safeEvaluate( + const runJS: PleasantestUtils['runJS'] = (code, args) => + asyncHookTracker.addHook(async () => { + // For some reason encodeURIComponent doesn't encode ' + const encodedCode = encodeURIComponent(code).replace(/'/g, '%27'); + const buildStatus = createBuildStatusTracker(); + // This uses the testPath as the url so that if there are relative imports + // in the inline code, the relative imports are resolved relative to the test file + const url = `http://localhost:${port}/${testPath}?inline-code=${encodedCode}&build-id=${buildStatus.buildId}`; + const res = await page.evaluate( + new Function( + '...args', + `return import(${JSON.stringify(url)}) + .then(async m => { + if (m.default) await m.default(...args) + }) + .catch(e => + e instanceof Error + ? { message: e.message, stack: e.stack } + : e)`, + ) as () => any, + ...(Array.isArray(args) ? (args as any) : []), + ); + + const errorsFromBuild = buildStatus.complete(); + // It only throws the first one but that is probably OK + if (errorsFromBuild) throw errorsFromBuild[0]; + + await sourceMapErrorFromBrowser(res, requestCache, port, runJS); + }, runJS); + + const injectHTML: PleasantestUtils['injectHTML'] = (html) => + asyncHookTracker.addHook( + () => + page.evaluate((html) => { + document.body.innerHTML = html; + }, html), injectHTML, - (html) => { - document.body.innerHTML = html; - }, - html, ); - }; - const injectCSS: PleasantestUtils['injectCSS'] = async (css) => { - await safeEvaluate( + const injectCSS: PleasantestUtils['injectCSS'] = (css) => + asyncHookTracker.addHook( + () => + page.evaluate((css) => { + const styleTag = document.createElement('style'); + styleTag.innerHTML = css; + document.head.append(styleTag); + }, css), injectCSS, - (css) => { - const styleTag = document.createElement('style'); - styleTag.innerHTML = css; - document.head.append(styleTag); - }, - css, ); - }; - const loadCSS: PleasantestUtils['loadCSS'] = async (cssPath) => { - const fullPath = isAbsolute(cssPath) - ? relative(process.cwd(), cssPath) - : join(dirname(testPath), cssPath); - await safeEvaluate( - loadCSS, - `import(${JSON.stringify( - `http://localhost:${port}/${fullPath}?import`, - )})`, - ); - }; - - const loadJS: PleasantestUtils['loadJS'] = async (jsPath) => { - const fullPath = jsPath.startsWith('.') - ? join(dirname(testPath), jsPath) - : jsPath; - const buildStatus = createBuildStatusTracker(); - const url = `http://localhost:${port}/${fullPath}?build-id=${buildStatus.buildId}`; - const res = await safeEvaluate( - loadJS, - `import(${JSON.stringify(url)}) - .then(mod => {}) - .catch(e => e instanceof Error - ? { message: e.message, stack: e.stack } - : e)`, - ); + const loadCSS: PleasantestUtils['loadCSS'] = (cssPath) => + asyncHookTracker.addHook(async () => { + const fullPath = isAbsolute(cssPath) + ? relative(process.cwd(), cssPath) + : join(dirname(testPath), cssPath); + await page.evaluate( + `import(${JSON.stringify( + `http://localhost:${port}/${fullPath}?import`, + )})`, + ); + }, loadCSS); + + const loadJS: PleasantestUtils['loadJS'] = (jsPath) => + asyncHookTracker.addHook(async () => { + const fullPath = jsPath.startsWith('.') + ? join(dirname(testPath), jsPath) + : jsPath; + const buildStatus = createBuildStatusTracker(); + const url = `http://localhost:${port}/${fullPath}?build-id=${buildStatus.buildId}`; + const res = await page.evaluate( + `import(${JSON.stringify(url)}) + .then(mod => {}) + .catch(e => e instanceof Error + ? { message: e.message, stack: e.stack } + : e)`, + ); - const errorsFromBuild = buildStatus.complete(); - // It only throws the first one but that is probably OK - if (errorsFromBuild) throw errorsFromBuild[0]; + const errorsFromBuild = buildStatus.complete(); + // It only throws the first one but that is probably OK + if (errorsFromBuild) throw errorsFromBuild[0]; - await sourceMapErrorFromBrowser(res, requestCache, port, loadJS); - }; + await sourceMapErrorFromBrowser(res, requestCache, port, loadJS); + }, loadJS); const utils: PleasantestUtils = { runJS, @@ -404,14 +389,14 @@ const createTab = async ({ loadJS, }; - const screen = getQueriesForElement(page, state); + const screen = getQueriesForElement(page, asyncHookTracker); // The | null is so you can pass directly the result of page.$() which returns null if not found const within: PleasantestContext['within'] = ( element: puppeteer.ElementHandle | null, ) => { assertElementHandle(element, within); - return getQueriesForElement(page, state, element); + return getQueriesForElement(page, asyncHookTracker, element); }; return { @@ -419,8 +404,8 @@ const createTab = async ({ utils, page, within, - user: await pleasantestUser(page, state), - state, + user: await pleasantestUser(page, asyncHookTracker), + asyncHookTracker, cleanupServer: () => closeServer(), }; }; diff --git a/src/pptr-testing-library.ts b/src/pptr-testing-library.ts index 41c315fc..38ee4911 100644 --- a/src/pptr-testing-library.ts +++ b/src/pptr-testing-library.ts @@ -2,6 +2,7 @@ import type { queries, BoundFunctions } from '@testing-library/dom'; import { jsHandleToArray, removeFuncFromStackTrace } from './utils'; import type { JSHandle } from 'puppeteer'; import { createClientRuntimeServer } from './module-server/client-runtime-server'; +import type { AsyncHookTracker } from './async-hooks'; type ElementToElementHandle = Input extends Element ? import('puppeteer').ElementHandle @@ -80,7 +81,7 @@ export type BoundQueries = BoundFunctions; export const getQueriesForElement = ( page: import('puppeteer').Page, - state: { isTestFinished: boolean }, + asyncHookTracker: AsyncHookTracker, element?: import('puppeteer').ElementHandle, ) => { const serverPromise = createClientRuntimeServer(); @@ -99,65 +100,46 @@ export const getQueriesForElement = ( return value; }); - const forgotAwait = removeFuncFromStackTrace( - new Error( - `Cannot execute query ${queryName} after test finishes. Did you forget to await?`, - ), - query, - ); - /** Handle error case for Target Closed error (forgot to await) */ - const handleExecutionAfterTestFinished = (error: any) => { - if (/target closed/i.test(error.message) && state.isTestFinished) { - throw forgotAwait; - } - - throw error; - }; const { port } = await serverPromise; const result: JSHandle = - await page - .evaluateHandle( - // Using new Function to avoid babel transpiling the import - // @ts-expect-error pptr's types don't like new Function - new Function( - 'argsString', - 'element', - `return import("http://localhost:${port}/@pleasantest/dom-testing-library") - .then(async ({ reviveElementsInString, printElement, addToElementCache, ...dtl }) => { - const deserializedArgs = JSON.parse(argsString, (key, value) => { - if (value.__serialized === 'RegExp') - return new RegExp(value.source, value.flags) - return value - }) - try { - return await dtl.${queryName}(element, ...deserializedArgs) - } catch (error) { - const message = - error.message + - (error.container - ? '\\n\\nWithin: ' + addToElementCache(error.container) - : '') - const messageWithElementsRevived = reviveElementsInString(message) - const messageWithElementsStringified = messageWithElementsRevived - .map(el => { - if (el instanceof Element || el instanceof Document) - return printElement(el) - return el - }) - .join('') - return { failed: true, messageWithElementsRevived, messageWithElementsStringified } - } - })`, - ), - serializedArgs, - element?.asElement() || - (await page - .evaluateHandle(() => document) - .catch(handleExecutionAfterTestFinished)), - ) - .catch(handleExecutionAfterTestFinished); + await page.evaluateHandle( + // Using new Function to avoid babel transpiling the import + // @ts-expect-error pptr's types don't like new Function + new Function( + 'argsString', + 'element', + `return import("http://localhost:${port}/@pleasantest/dom-testing-library") + .then(async ({ reviveElementsInString, printElement, addToElementCache, ...dtl }) => { + const deserializedArgs = JSON.parse(argsString, (key, value) => { + if (value.__serialized === 'RegExp') + return new RegExp(value.source, value.flags) + return value + }) + try { + return await dtl.${queryName}(element, ...deserializedArgs) + } catch (error) { + const message = + error.message + + (error.container + ? '\\n\\nWithin: ' + addToElementCache(error.container) + : '') + const messageWithElementsRevived = reviveElementsInString(message) + const messageWithElementsStringified = messageWithElementsRevived + .map(el => { + if (el instanceof Element || el instanceof Document) + return printElement(el) + return el + }) + .join('') + return { failed: true, messageWithElementsRevived, messageWithElementsStringified } + } + })`, + ), + serializedArgs, + element?.asElement() || (await page.evaluateHandle(() => document)), + ); const failed = await result.evaluate( (r) => typeof r === 'object' && r !== null && (r as DTLError).failed, @@ -174,12 +156,8 @@ export const getQueriesForElement = ( const error = new Error(messageWithElementsStringified); // @ts-expect-error messageForBrowser is a custom property that we add to Errors error.messageForBrowser = messageWithElementsRevived; - // Manipulate the stack trace and remove this function - // That way Jest will show a code frame from the user's code, not ours - // https://kentcdodds.com/blog/improve-test-error-messages-of-your-abstractions - Error.captureStackTrace(error, query); - throw error; + throw removeFuncFromStackTrace(error, query); } // If it returns a JSHandle, make it into an array of JSHandles so that using [0] for getAllBy* queries works @@ -202,7 +180,11 @@ export const getQueriesForElement = ( return result.jsonValue(); }; - return [queryName, query]; + return [ + queryName, + (...args: any[]) => + asyncHookTracker.addHook(() => query(...args), queries[queryName]), + ]; }), ); diff --git a/src/user.ts b/src/user.ts index 9f99573f..9549f9c1 100644 --- a/src/user.ts +++ b/src/user.ts @@ -1,4 +1,5 @@ import type { ElementHandle, JSHandle, Page } from 'puppeteer'; +import type { AsyncHookTracker } from './async-hooks'; import { createClientRuntimeServer } from './module-server/client-runtime-server'; import { assertElementHandle, @@ -31,31 +32,16 @@ export interface PleasantestUser { ): Promise; } -const forgotAwaitMsg = - 'Cannot interact with browser after test finishes. Did you forget to await?'; - /** Wraps each user method to catch errors that happen when user forgets to await */ const wrapWithForgotAwait = ( user: PleasantestUser, - state: { isTestFinished: boolean }, + asyncHookTracker: AsyncHookTracker, ) => { for (const key of Object.keys(user) as (keyof PleasantestUser)[]) { const original = user[key]; // eslint-disable-next-line @cloudfour/unicorn/consistent-function-scoping - const wrapper = async (...args: any[]) => { - const forgotAwaitError = removeFuncFromStackTrace( - new Error(forgotAwaitMsg), - wrapper, - ); - - try { - return await (original as any)(...args); - } catch (error) { - throw state.isTestFinished && /target closed/i.test(error.message) - ? forgotAwaitError - : error; - } - }; + const wrapper = (...args: any[]) => + asyncHookTracker.addHook(() => (original as any)(...args), wrapper); user[key] = wrapper; } @@ -63,7 +49,7 @@ const wrapWithForgotAwait = ( export const pleasantestUser = async ( page: Page, - state: { isTestFinished: boolean }, + asyncHookTracker: AsyncHookTracker, ) => { const { port } = await createClientRuntimeServer(); const runWithUtils = ( @@ -219,7 +205,7 @@ Element must be an or