Skip to content
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: expand prettier-e2e test and update typings/deps #14779

Merged
merged 4 commits into from Jul 26, 2022

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Jul 21, 2022

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

1.#14765 (comment)

2.#13414 (comment)

3.#14777 (comment)

4.#14701 (comment)

@liuxingbaoyu liuxingbaoyu added the PR: Internal 🏠 A type of pull request used for our changelog categories label Jul 21, 2022
@babel-bot
Copy link
Collaborator

babel-bot commented Jul 21, 2022

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52614/

@liuxingbaoyu
Copy link
Member Author

#14723 (plugin-transform-typescript) was released at v7.18.8, but the latest version of https://www.npmjs.com/package/@babel/preset-typescript is v7.18.6.😓

@@ -40,6 +40,6 @@ yarn install

# Only run js,jsx,misc format tests
# Without --runInBand CircleCI hangs.
yarn test "tests/format/(jsx?|misc)/" --update-snapshot --runInBand
yarn test "tests/format/(jsx?|misc|typescript|flow|flow-repo)/" --update-snapshot --maxWorkers=2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have to defer this change until prettier updated the snapshot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll wait for prettier/prettier#13065.

Copy link
Contributor

@fisker fisker Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't work... I'm fixing that in next branch

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you switch to next? We'll focus on this branch for a while. And if you do that --maxWorkers should be removed, because we use --runInBand, have to do that with jest-light-runner.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried this will be a little slow.😕

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a recoverable exception, it doesn't throw, and ast is generated normally, maybe that has something to do with it?

Copy link
Contributor

@fisker fisker Jul 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we allow some recoverable error https://github.com/prettier/prettier/blob/ab72a2c11c806f3a8a5ef42314e291843e1b3e68/src/language-js/parse/babel.js#L183, have you changed the message code or throwing a different message code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, we can fix that test by disallowing that from recovering( make it invalid ), it's invalid in typescript parser after all.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@liuxingbaoyu
Copy link
Member Author

#14723 (plugin-transform-typescript) was released at v7.18.8, but the latest version of npmjs.com/package/@babel/preset-typescript is v7.18.6.😓

@nicolo-ribaudo Can you take a look at this? I'm not sure how I should update, using resolutions doesn't seem elegant.

@nicolo-ribaudo
Copy link
Member

Delete @babel/plugin-transform-typescript from the lockfile and run yarn

@liuxingbaoyu
Copy link
Member Author

Oh, thanks! (I didn't think of it at all😂)

@JLHwung JLHwung changed the title chore: lots of chores chore: expand prettier-e2e test and update typings/deps Jul 26, 2022
@JLHwung JLHwung merged commit bfb4f98 into babel:main Jul 26, 2022
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 27, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants