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

feat(babel-plugin-jsx-pragmatic): migrate to typescript #2570

Merged
merged 4 commits into from
Mar 18, 2022
Merged

feat(babel-plugin-jsx-pragmatic): migrate to typescript #2570

merged 4 commits into from
Mar 18, 2022

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Nov 27, 2021

What:

Migrates @emotion/babel-plugin-jsx-pragmatic to TypeScript.

Why:

Part of #2358

How:

Renamed the js file to ts, and made changes to make TS happy.

Checklist:

  • Documentation
  • Tests
  • Code complete
  • Changeset

I've not converted the tests because I don't know how to handle babel-tester since it's apparently coming in as some magical workspace thing (I've not worked with workspaces a lot 😅).

I'm sure there will be a way to tell TypeScript that it exists, but that probably will need either a type definition written or the package converted to TypeScript, so have left it for now.

I'll be making a few follow-up PRs to DefinitelyTyped that'll remove the need for some of the types, but they probably won't land for at least a month.

@changeset-bot
Copy link

changeset-bot bot commented Nov 27, 2021

🦋 Changeset detected

Latest commit: 7a491b3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@emotion/babel-plugin-jsx-pragmatic Minor
@emotion/babel-preset-css-prop Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 27, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7a491b3:

Sandbox Source
Emotion Configuration

@G-Rath
Copy link
Contributor Author

G-Rath commented Nov 27, 2021

btw this should also allow me to do @emotion/preset-css-prop, since that uses this package

@codecov
Copy link

codecov bot commented Nov 27, 2021

Codecov Report

Merging #2570 (29d6afe) into ts-migration (85772c3) will decrease coverage by 0.03%.
The diff coverage is 75.00%.

❗ Current head 29d6afe differs from pull request most recent head 7a491b3. Consider uploading reports for the commit 7a491b3 to get more accurate results

Impacted Files Coverage Δ
packages/babel-plugin-jsx-pragmatic/src/index.ts 92.00% <75.00%> (ø)

Copy link
Member

@srmagura srmagura left a comment

Choose a reason for hiding this comment

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

@G-Rath Looks good. Was there any progress on getting some of these types into DefinitelyTyped so they can be removed from the Emotion code?

Edit: I see you now that you got the PluginPass stuff merged in DefinitelyTyped. Could you update this PR to take advantage of that change? Thanks.

Edit 2: Ideally the tests for babel-plugin-jsx-pragmatic should be converted to TypeScript too for this change. The code for babel-tester is in scripts/babel-tester/src/index.js. If you change that file to .ts it should "just work".

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 19, 2022

Ideally the tests for babel-plugin-jsx-pragmatic should be converted to TypeScript too for this change.

I don't mind doing converting babel-tester & the tests for this to TypeScript, but would prefer to address it in another PR if possible because I think it is a larger change: it's types will probably need some crafting, and at the least babel-check-duplicated-nodes doesn't have types so someone will need to define them.

@srmagura
Copy link
Member

OK, since it is a bigger change, doing it in a separate PR is fine 👍

@Andarist I believe this is ready to be merged.

@srmagura srmagura self-requested a review February 19, 2022 23:31
Comment on lines +5604 to +5606
"@types/babel__generator" "*"
"@types/babel__template" "*"
"@types/babel__traverse" "*"
Copy link
Member

Choose a reason for hiding this comment

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

sweet, sweet little lies 😅

@Andarist
Copy link
Member

@G-Rath I've prepared babel-tester migration here: #2688

@Andarist Andarist merged commit ea84c40 into emotion-js:ts-migration Mar 18, 2022
@G-Rath G-Rath deleted the migrate-babel-plugin-jsx-pragmatic-to-typescript branch March 19, 2022 00:01
@github-actions github-actions bot mentioned this pull request May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants