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: add package.json options to Related #2897

Merged
merged 2 commits into from Jul 24, 2023

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Jul 18, 2023

Summary

New features like customConditions, resolvePackageJsonExports, and resolvePackageJsonImports are Related to moduleResolution

Details

- these influence how `moduleResolution` works, and so should be Related to it
- and also Related to each other
@agilgur5
Copy link
Contributor Author

Alright think this is my last PR for the time being 😅

@jakebailey
Copy link
Member

Hm, by this argument, I'm pretty sure there are loads of other options that affect module resolution, like suffixes, paths, json modules, etc; where do we draw the line?

@agilgur5
Copy link
Contributor Author

agilgur5 commented Jul 18, 2023

It's a good question, I thought about that too. I'm not opposed to adding more relatedTo actually.
These 3 just are particularly relevant in the node16+ world.

I actually didn't know these options existed -- and thought that changing moduleResolution was the only way to get these to work (one of the reasons why we prioritized ezolenko/rollup-plugin-typescript2#453) -- until I read through the reference more recently.

In a similar line of thought, could argue that these 3 specifically are impacted by moduleResolution's setting, whereas the others are not necessarily impacted (or not so directly).
Again though, I'd be fine adding more relatedTo because they are all relevant for different use cases; node16+ is just more relevant to a wide audience (the vast majority of TS users, I would guess) and is timely right now

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

I think the completeness of these lists is bound to be a little arbitrary, but I like the idea of increasing the discoverability of these lesser-known related options. I’ve added my own suggestions to the list, as well as some to module.

packages/tsconfig-reference/scripts/tsconfigRules.ts Outdated Show resolved Hide resolved
packages/tsconfig-reference/scripts/tsconfigRules.ts Outdated Show resolved Hide resolved
@andrewbranch andrewbranch merged commit 7601eca into microsoft:v2 Jul 24, 2023
5 checks passed
@agilgur5 agilgur5 deleted the feat-relatedto-package-json branch July 24, 2023 21:13
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.

None yet

3 participants