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: Allow for automatic ts mapping detection #114

Merged
merged 6 commits into from
Sep 11, 2023

Conversation

scagood
Copy link

@scagood scagood commented Sep 4, 2023

This adds the ability to automatically detect the correct ts file mappings from the tsconfig.json

@scagood

This comment was marked as outdated.

@scagood
Copy link
Author

scagood commented Sep 4, 2023

Also, I am not much of a front ender, so if anyone who is more versed in the jsx, tsx world could look (and likely spot some issues)... please do let me know 😅

@scagood
Copy link
Author

scagood commented Sep 5, 2023

@scagood

This comment was marked as resolved.

@aladdin-add
Copy link

I've re-run the ci - all green now. 🎉

@scagood scagood marked this pull request as ready for review September 10, 2023 22:09
@scagood
Copy link
Author

scagood commented Sep 10, 2023

I have made a shared-settings.md gist here: https://gist.github.com/scagood/0879d7e51b4e1cda13eedd2f8d879c97

I want to know if that is the sort of thing you had in mind? I tried my best to find all the shared config settings, I think I got them all! 👀

I am thinking that if it is, it may be worth putting in a seperate PR as I would need to update a fair few rule docs.

Copy link

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for doing this! ❤️

@@ -156,59 +156,133 @@ ruleTester.run("no-missing-import", rule, {

// typescriptExtensionMap
{
// name: "settings.node - [] as react - d.ts as d.js",

Choose a reason for hiding this comment

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

👍 I like having the name field, it does not have to be in comments - per the eslint docs?

Copy link
Author

Choose a reason for hiding this comment

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

It actually does because the eslint v7 tests throw an error 😢

I ended up commenting them here: b0de4ae (#114)

Choose a reason for hiding this comment

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

looks like it was introduced in an eslint v8 release. 😄

@aladdin-add aladdin-add merged commit 2ab30ce into eslint-community:master Sep 11, 2023
9 checks passed
@aladdin-add
Copy link

I'm merging this now, the doc changes can be in another PR.

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