Adds auto-remove types to plugin-typescript

[deini]

@arcanis I'm trying to implement a prompt for plugin-typescript but I have a ton of questions 😂

Questions:

1. Is there a way to have access to preferInteractive inside afterWorkspaceDependencyAddition hook? I've read in the plugin's README that ideally it should be using preferInteractive to either prompt or not (if I'm understanding it correctly).
2. If we use preferInteractive would we expect to only add @types when prompted or would we also add them by default?
3. Right now we always use EXACT for @types, is there a reason for that or should it be changed to CARET?
4. The target is always REGULAR, would it make more sense to add it to DEVELOPMENT? Or match the dependencyTarget?
5. What do you think of also hooking into afterWorkspaceDependencyRemoval and remove types?
6. Since this is not a plugin that adds a command not sure how to go about testing it 🤔. Could I potentially setup something similar to the add tests?
7. Will leave a few questions as PR comments.

[deini]

Should the types be here? 🤔

[arcanis]

Types probably should be declared at the place the typeRoots configuration is set. At the moment it's at the root of the repository.

@Vlasenko's work will likely remove the need for typesRoot in the medium term, which would make it possible to store the types in each individual package.

[arcanis]

1. Is there a way to have access to preferInteractive inside afterWorkspaceDependencyAddition hook? I've read in the plugin's README that ideally it should be using preferInteractive to either prompt or not (if I'm understanding it correctly).

Yep! The Workspace class has a reference to its parent Project, which has a configuration to its parent Configuration. So it would be workspace.project.configuration.get(`preferInteractive`).

2. If we use preferInteractive would we expect to only add @types when prompted or would we also add them by default?

I'd tend to say that we would always want to add it - it doesn't cost much to have too many @types packages, and the case of "I have a dependency in a TS project but don't want the types" must be relatively rare 🤔

3. Right now we always use EXACT for @types, is there a reason for that or should it be changed to CARET?

iirc the @types packages don't follow semver. More details on this discussion:

yarnpkg/yarn#6695 (comment)

4. The target is always REGULAR, would it make more sense to add it to DEVELOPMENT? Or match the dependencyTarget?

Adding them to the devDependencies makes sense! It's probably a bit overkill to match the exact target - I guess it's something that we'll have to see at usage.

5. What do you think of also hooking into afterWorkspaceDependencyRemoval and remove types?

Sounds good to me 👍

6. Since this is not a plugin that adds a command not sure how to go about testing it 🤔. Could I potentially setup something similar to the add tests?

Plugin tests are currently a bit weird.

• In theory it would be nice if the tests for each individual plugin could live inside their own folders.
• But for now it's not really possible (there isn't that much work since pkg-tests-core is a workspace, but the plumbing for the Jest config isn't trivial), so you can just create a file in acceptance-tests/pkg-tests-specs/sources/plugins (this folder doesn't exist yet), for example plugin-typescript.ts.

[deini]

Not sure if I should use makeIdent or something like structUtils.parseIdent(typesName).

[arcanis]

Better to use makeIdent for names you generate; parseIdent is mostly meant to be used with user input (like CLI parameters, for example).

[deini]

I'm now checking preferInteractive, however, I'm not sure how to check if yarn add is running with --interactive. 🤔

[arcanis]

One question I have regarding the goal of the PR - in my mind the @types packages were automatically added / removed so that the user didn't have to care about them. Doesn't adding a prompt go against this principle? In which situation will it benefit the user to say "no, I don't want to add the types for X" or "no, I don't want to remove the types for Y"?

[deini]

I have the same question. I am basing the work on this comment:

This is a crude implementation so far (an ideal implementation should use preferInteractive in order to ask the user for confirmation), but it gives an idea of how hooks work. Help appreciated!

🤔 Maybe when you add something like prettier or eslint and plan to use only their cli?

[arcanis]

I have the same question. I am basing the work on this comment

Well that's awkward since I wrote this line myself 😅 I probably didn't completely thought it through when I wrote it ...

However the "auto-remove" part is useful, can you update your PR to only keep this part for now? Plus we sidestep the problem with --interactive! 😄

[arcanis]

Sounds good! I'll merge it as it is, feel free to followup on the stringifyIdent thing whenever you like (if you can add a test or two it would also be awesome 😃).

[arcanis]

Technically this probably should be structUtils.stringifyIdent