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

Consider changing rules for determining whether a project uses TS #22018

Closed
tgriesser opened this issue Jun 1, 2022 · 6 comments · Fixed by #22058
Closed

Consider changing rules for determining whether a project uses TS #22018

tgriesser opened this issue Jun 1, 2022 · 6 comments · Fixed by #22058
Assignees
Labels
CT Issue related to component testing type: bug

Comments

@tgriesser
Copy link
Member

tgriesser commented Jun 1, 2022

What would you like?

From #22004

I don’t have a tsconfig, don’t use typescript in the project, I have a tsconfig.json.ignore from back when I did, but that’s clearly nothing

We should probably tighten the algorithm for determining whether to use ts or not in a project from:

IF `typescript` dependency in `package.json` OR `tsconfig.json` in `projectRoot/*`

to:

IF `typescript` dependency in `package.json` AND `tsconfig.json` in `projectRoot/*`

We should be detecting via existence in dependencies / devDependencies / peerDependencies as well as resolving it to confirm it's usable

cc @ZachJW34 / @lmiller1990 as I think y'all worked on this one the most.

Why is this needed?

No response

Other

Related: #21927

@jennifer-shehane
Copy link
Member

Let's make sure to update the tech-brief if we change this logic.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jun 2, 2022

We should be detecting via existence in dependencies / devDependencies / peerDependencies rather than resolving it

I don't think we are resolving it anywhere during detection. What gave you this impression?

We can easily make the suggested change. I don't think it'll solve any existing problems, though - tsconfig.json.ignore won't be detected by our glob:

joinPosix('**/*tsconfig.json'),

I think there are some outstanding issues in #22004 that are not related to the TS detection algorithm.

@lmiller1990
Copy link
Contributor

Also @tgriesser I wonder if we want to rethink the brief a little as part of this. I made a PR: https://github.com/cypress-io/tech-briefs/pull/28. What do you think?

@tgriesser
Copy link
Member Author

Yeah, let's make this change - seems it's a common enough issue that folks are running into

@tgriesser
Copy link
Member Author

Actually, we probably should also be resolving it, because if they don't have it installed it'll immediately error for them when they scaffold

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 2, 2022

Released in 10.0.2.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v10.0.2, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 2, 2022
@mjhenkes mjhenkes removed the PATCH label Jun 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CT Issue related to component testing type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants