feat: export configure function with data-testid override #39
Conversation
There was a problem hiding this comment.
clever hack for this @silviuaavram thanks for the contribution! the suggestions should help with the lint errors you're seeing, but the overall impl works for me :)
lib/index.ts
Outdated
| @@ -17,7 +17,7 @@ function mapArgument(argument: any, index: number): any { | |||
| : argument | |||
| } | |||
|
|
|||
| const delegateFnBodyToExecuteInPage = ` | |||
| let delegateFnBodyToExecuteInPage = ` | |||
There was a problem hiding this comment.
let's save the original as const delegateFnBody so we can configure more than once :)
lib/index.ts
Outdated
| @@ -130,6 +130,21 @@ export function wait( | |||
| return waitForExpect(callback, timeout, interval) | |||
| } | |||
|
|
|||
| export function configure(newConfig: Partial<Config>) { | |||
There was a problem hiding this comment.
| export function configure(newConfig: Partial<Config>) { | |
| export function configure(options: Partial<IConfigureOptions>): void { |
lib/typedefs.ts
Outdated
| @@ -58,3 +58,7 @@ export interface IQueryUtils extends IQueryMethods { | |||
| getQueriesForElement(): IQueryUtils & IScopedQueryUtils | |||
| getNodeText(el: Element): Promise<string> | |||
| } | |||
|
|
|||
| export interface Config { | |||
There was a problem hiding this comment.
| export interface Config { | |
| export interface IConfigureOptions { |
lib/index.ts
Outdated
| @@ -3,7 +3,7 @@ import * as path from 'path' | |||
| import {ElementHandle, EvaluateFn, JSHandle, Page} from 'puppeteer' | |||
| import waitForExpect from 'wait-for-expect' | |||
|
|
|||
| import {IQueryUtils, IScopedQueryUtils} from './typedefs' | |||
| import {IQueryUtils, IScopedQueryUtils, Config} from './typedefs' | |||
There was a problem hiding this comment.
| import {IQueryUtils, IScopedQueryUtils, Config} from './typedefs' | |
| import {IConfigureOptions, IQueryUtils, IScopedQueryUtils} from './typedefs' |
lib/index.ts
Outdated
| @@ -130,6 +130,21 @@ export function wait( | |||
| return waitForExpect(callback, timeout, interval) | |||
| } | |||
|
|
|||
| export function configure(newConfig: Partial<Config>) { | |||
| const { testIdAttribute } = newConfig; | |||
There was a problem hiding this comment.
| const { testIdAttribute } = newConfig; | |
| const {testIdAttribute} = options |
lib/index.ts
Outdated
| const { testIdAttribute } = newConfig; | ||
|
|
||
| if ( | ||
| testIdAttribute && |
There was a problem hiding this comment.
| testIdAttribute && |
this one is redundant with the other checks :)
lib/index.ts
Outdated
| testIdAttribute !== '' | ||
| ) { | ||
| delegateFnBodyToExecuteInPage = delegateFnBodyToExecuteInPage.replace( | ||
| `testIdAttribute: 'data-testid'`, |
There was a problem hiding this comment.
does this pattern only ever occur once or do we need a global replace?
|
Thank you for the review @patrickhulce and for getting back so soon, I will address the comments! I was not sure about the solutions so I wanted to create a PoC for it. All the suggestion make sense, thanks! Will ping you once I finish the improvements. |
|
@patrickhulce it's done, let me know what you think. It's not ideal, as string replacing in this case is fragile, but it's how we can get it done with minimal changes + we are covered by unit tests, just in case we update dom testing library and there are changes to the strings we replace. |
|
SG @silviuaavram this looks great to me if you could just cleanup the lint errors! |
|
This is cool, thank you so much for the feedback! Looking forward to seeing it released and used! |
|
published in 0.6.2 :) |
Closes #32.
Replaces all occurrences of
testIdAttribute: 'data-testid'withtestIdAttribute: '<custom-attribute-name>'in the imported stringifieddom-testing-library. Also keeps the original string just in case we need to perform subsequent changes.Adds 2 unit tests to cover the changes.
Adds a new type that accepts only the supported parameter from
configure.