Add optional parameter to createAppTester to customize storeKey #147
Conversation
michellechu77
commented
Mar 28, 2019
michellechu77
commented
- Updated index.d.ts
|
@xavdid, wasn't too familiar with the index.dt.s file format, but took a guess at it. |
src/tools/create-app-tester.js
Outdated
| @@ -36,7 +36,7 @@ const promisifyHandler = handler => { | |||
| }; | |||
|
|
|||
| // A shorthand compatible wrapper for testing. | |||
| const createAppTester = appRaw => { | |||
| const createAppTester = (appRaw, storeKey = null) => { | |||
There was a problem hiding this comment.
this is good, but typically it's good to avoid null. It behaves weirdly with default args.
That said, you don't actually have to set a default here. The verbose version of this function signature is (appRaw, storeKey=undefined), but since js doesn't really enforce arguments at all, it's most clearly written as (appRaw, storeKey). Does that make sense?
There was a problem hiding this comment.
I see now that you likely copied the null from below. Honestly I shouldn't have written it like that in the first place. 😁
src/tools/create-app-tester.js
Outdated
| @@ -46,14 +46,15 @@ const createAppTester = appRaw => { | |||
| bundle = bundle || {}; | |||
|
|
|||
| const method = resolveMethodPath(appRaw, methodOrFunc); | |||
| let myStoreKey = storeKey ? `testKey-${storeKey}` : `testKey-${method}-${randomSeed}` | |||
There was a problem hiding this comment.
though it's perfect for quick adhoc use, i'd probably name this variable something else. maybe customStoreKey.
That said, looking below, I'd probably move the ternary up here and rename that variable. So the whole thing would be
(appRaw, customStoreKey) =>
// ...
const storeKey = shouldPaginate(appRaw, method)
? storeKey
? `testKey-${storeKey}`
: `testKey-${method}-${randomSeed}`
: undefined;
// ...
method,
bundle,
storeKey
};if you go that route, remember to adjust the index.d.ts appropriately.
There was a problem hiding this comment.
good idea, I like your suggested variable name better.
