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

Type checking all repos requires npm install in many repos #428

Open
zepumph opened this issue Jan 27, 2025 · 3 comments
Open

Type checking all repos requires npm install in many repos #428

zepumph opened this issue Jan 27, 2025 · 3 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Jan 27, 2025

As a project we are running into more and more cases where a project has an npm dependency of @types/* and thus requires an npm install in order for type checking to work.

Paper trails:

I believe we should have a discussion about this, because it seems obvious that the best long term solution is to require an npm install on any repo that you are trying to type check. That seems idiomatic and nice, but we don't want to cause too much burden to all devs for this.

This brings up other, scope-creepy-kinds of questions:

  • Do we need to keep using npm
  • Should we use package-lock.json files
  • Can we get rid of the dependency we have on grunt in every repo, therefore making most simulation repos not need an npm install step.

I'm creating this issue because we want to have a good solution to this. Currently, over in rosetta, the recommendation @samreid and I gave to @jbphet was just to use @ts-expect-errors on any type errors that are caused from dependency type issues. Booooo.

I believe the people that should be involved in this meeting are:
@jonathanolson @samreid @zepumph and any other parties that are interested.

@zepumph zepumph self-assigned this Jan 27, 2025
@zepumph
Copy link
Member Author

zepumph commented Jan 27, 2025

I'll start by assigning to @jonathanolson for comment.

@jonathanolson
Copy link
Contributor

Do we need to keep using npm

No, but we could switch to similar things, like pnpm or yarn.

Should we use package-lock.json files

Probably for certain things, yes! This is helpful for the scenerystack build, ideally we should be keeping dependencies stable instead of having them ping around.

Can we get rid of the dependency we have on grunt in every repo, therefore making most simulation repos not need an npm install step.

That would be great, but I don't know about how possible it is.

All of that said, I think if we have developers consistently using a script (like main-pull-status) that takes care of these things, then we won't disrupt things as much whenever repos get added to a "npm update" list.

@zepumph
Copy link
Member Author

zepumph commented Feb 4, 2025

Looking through current usages of npm updates, there are many spots that assume that for a given sim to run, you need to update in that sim repo, perennial-alias, and chipper. This issue could potentially break that assumption, like for phetsims/aqua#228, but to start I think I'll focus on just updates to main, working through #405.

zepumph added a commit that referenced this issue Feb 5, 2025
zepumph added a commit that referenced this issue Feb 5, 2025
zepumph added a commit that referenced this issue Feb 5, 2025
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

No branches or pull requests

2 participants