-
Notifications
You must be signed in to change notification settings - Fork 906
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
chore: migrate upgrade command to TS and remove legacy implementation #684
Conversation
52ec050
to
bc6760d
Compare
"typescript": "^3.6.2" | ||
}, | ||
"devDependencies": { | ||
"@babel/plugin-proposal-class-properties": "^7.5.5" |
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.
Why we need it?
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.
tests are failing without this transform, not yet sure why
).toMatchSnapshot( | ||
'RnDiffApp is replaced with app name (TestApp and com.testapp)', | ||
); | ||
}, 60000); | ||
test('fetches regular patch, adds remote, applies patch, installs deps, removes remote when updated from nested directory', async () => { | ||
mockFetch(samplePatch, 200); | ||
(execa: any).mockImplementation(mockExecaNested); | ||
((execa as unknown) as jest.Mock).mockImplementation(mockExecaNested); |
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.
I usually make it ((execa as any) as jest.Mock)
, which might be a little bit smaller. It doesn't really matter here :P
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.
Seems legit. Those // $FlowFixMe
are a little bit awkward, but I don't see any other solution to migrating module piece by piece.
Summary:
Part of #683
Had issues with inquirer module typings, they somehow clash with Node. Not sure how to resolve, help wanted :)
Btw decided to remove legacy upgrade implementation, it's about time.
Test Plan:
Green CI