Skip to content

Conversation

@srmagura
Copy link
Contributor

This change was originally made here. Its purpose is to prevent the following error from Preconstruct.

🎁 error Generating TypeScript declarations for packages/eslint-plugin/src/index.ts failed:
🎁 error packages/eslint-plugin/src/index.ts:8:12 - error TS2742: The inferred type of 'rules' cannot be named without a reference to '@typescript-eslint/experimental-utils/node_modules/@typescript-eslint/types/dist/ast-spec'. This is likely not portable. A type annotation is necessary.
🎁 error
🎁 error 8 export let rules = {
🎁 error ~~~~~
🎁 error packages/eslint-plugin/src/index.ts:8:12 - error TS4023: Exported variable 'rules' has or is using name 'JSXConfig' from external module "/home/srmagura/Documents/oss/emotion-ts/packages/eslint-plugin/src/rules/jsx-import" but cannot be named.
🎁 error
🎁 error 8 export let rules = {
🎁 error ~~~~~
🎁 error

@changeset-bot
Copy link

changeset-bot bot commented May 21, 2022

⚠️ No Changeset found

Latest commit: 79848d3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@srmagura srmagura requested a review from Andarist May 21, 2022 15:00
@codesandbox-ci
Copy link

codesandbox-ci bot commented May 21, 2022

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 79848d3:

Sandbox Source
Emotion Configuration

@codecov
Copy link

codecov bot commented May 22, 2022

Codecov Report

Merging #2761 (79848d3) into ts-migration (8c4e2c3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
packages/eslint-plugin/src/rules/jsx-import.ts 98.00% <ø> (ø)
...kages/eslint-plugin/src/rules/syntax-preference.ts 91.81% <ø> (ø)
packages/eslint-plugin/src/utils.ts 100.00% <ø> (ø)
...ges/eslint-plugin/src/rules/import-from-emotion.ts 94.73% <100.00%> (+0.29%) ⬆️
packages/eslint-plugin/src/rules/no-vanilla.ts 100.00% <100.00%> (ø)
packages/eslint-plugin/src/rules/pkg-renaming.ts 92.00% <100.00%> (+0.33%) ⬆️
packages/eslint-plugin/src/rules/styled-import.ts 91.66% <100.00%> (+0.75%) ⬆️

@Andarist
Copy link
Member

@srmagura I've pushed out a different fix for this issue, could you take a look? I've diagnosed the core of the issue - you can read about it here: typescript-eslint/typescript-eslint#5032

@srmagura
Copy link
Contributor Author

Nice job, it looks great. I think it is ready for merge. When you merge, I'll create an issue reminding us to go back and remove the explicitly-set generic parameters once typescript-eslint fixes the underlying bug.

@Andarist
Copy link
Member

I'll create an issue reminding us to go back and remove the explicitly-set generic parameters once typescript-eslint fixes the underlying bug.

That might only happen in their next major version - and the variant with explicitly-set generic parameters ain't technically incorrect anyway so I don't think the issue is necessary here to keep around an issue for the undetermined amount of time.

@Andarist Andarist merged commit 8ed0042 into emotion-js:ts-migration May 22, 2022
@srmagura srmagura deleted the eslint-ts-hacks branch July 10, 2022 14:36
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.

2 participants