ts-migration/no-large-snapshots #376
Conversation
| if ( | ||
| propertyName === 'toMatchInlineSnapshot' || | ||
| propertyName === 'toThrowErrorMatchingInlineSnapshot' | ||
| 'property' in node.callee && |
There was a problem hiding this comment.
@SimenB I know what you're going to say - I'm going to implement varargs in it's own PR, as it's a waterfall change that while relatively straightforward, does require a bit of its own work due to subtyping.
The main reason I replaced the array here is this way isSupportedAccessor has full coverage; otherwise I'd have left it as includes & done the varargs thing in another PR.
| export const getAccessorValue = <S extends string = string>( | ||
| accessor: AccessorNode<S>, | ||
| ): S => | ||
| /* istanbul ignore next */ |
There was a problem hiding this comment.
This is b/c we have no rules that use this on an Identifier so coverage complains, but no-large-snapshots uses isSupportedAccessor on an Identifier, meaning AccessorNode needs to be a union, and so TypeScript requires us to handle both branches.
It's a catch-22 :)
There was a problem hiding this comment.
you solved this another way? Seems to be gone
There was a problem hiding this comment.
I solved it by using it - this was never needed in this branch, but slipped in via the rebase
There was a problem hiding this comment.
Sorry thought this was #363 - my tests locally still require this for this PR?
As this point I'd like to keep it in just to avoid dealing w/ the conflict when PRing all the other rules.
G-Rath
force-pushed
the
ts-migration/no-large-snapshots
branch
from
70d382d to
75ad799
Compare
|
FYI I'm in the middle of rebasing this one :) |
G-Rath
force-pushed
the
ts-migration/no-large-snapshots
branch
from
75ad799 to
7173b4e
Compare
Heh, new conflict 😅 |
hehe yeah I'm going to finish the second round of rebasing, and then go to bed, cause I've got work in 5 hours, and these rebases are just going loopy 😂 I'll look to respond to your other review stuff "tomorrow", but in general from what I've read yes to everything :P |
|
Provided the tests push, I'm going to push up the rebased version. @SimenB if you've got the time, feel free to rebase the commits to make future rebasing easier while I'm sleeping. Right now it's actually pretty horrible, I think in part b/c there's a number of now redundant commits that are very steppy i.e rename method, move method, remove method -> end result is method that now already exists in master. Otherwise, I can try and remove them via rebase when I get to work. Cheers edit: these last two comments were actually about #363 😂 |
|
I can rebase #363 after landing this and see if I can clear out a few of the "chore" commits |
G-Rath
force-pushed
the
ts-migration/no-large-snapshots
branch
from
7173b4e to
7202e7d
Compare
|
🎉 This PR is included in version 22.15.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Based off #375