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

Upgrade to typescript-eslint 8.15 #413

Open
zepumph opened this issue Nov 21, 2024 · 4 comments
Open

Upgrade to typescript-eslint 8.15 #413

zepumph opened this issue Nov 21, 2024 · 4 comments

Comments

@zepumph
Copy link
Member

zepumph commented Nov 21, 2024

@mattpen reported trouble in slack:

I’m getting a lot of eslint errors like: Error while loading rule '@typescript-eslint/no-unused-expressions': Cannot read properties of undefined (reading 'allowShortCircuit'). It looks like this one at least is fixed in typescript-eslint 8.15.0, but we are using ~8.13.0 in perennial which pins us to that minor version. Can we consider using ^ instead of ~?

@zepumph zepumph self-assigned this Nov 21, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 21, 2024

Michael Kauzmann
2 minutes ago
One time a while ago we decided to pin minor versions to help with maintenance of older shas (because for reasons unknown to me we don't use package-locks). I think we should just upgrade to 8.15. I'll make an issue.

@zepumph
Copy link
Member Author

zepumph commented Nov 21, 2024

I wanted to just commit this, but I found that we get many errors in grunt check --all --clean that look like this:

error TS6305: Output file 'C:/Users/mjkauzmann/PHET/git/chipper/dist/tsc/outDir/kite/js/ShapeTests.d.ts' has not been built from source file 'C:/Users/mjkauzmann/PHET/git/kite/js/ShapeTests.js'.
  The file is in the program because:
    Matched by include pattern 'js/**/*' in 'C:/Users/mjkauzmann/PHET/git/kite/tsconfig-kite-browser.json'

  ../../../../kite/tsconfig-kite-browser.json:4:5
    4     "js/**/*",
          ~~~~~~~~~
    File is matched by include pattern specified here.

error TS6305: Output file 'C:/Users/mjkauzmann/PHET/git/chipper/dist/tsc/outDir/kite/js/kite-main.d.ts' has not been built from source file 'C:/Users/mjkauzmann/PHET/git/kite/js/kite-main.js'.
  The file is in the program because:
    Matched by include pattern 'js/**/*' in 'C:/Users/mjkauzmann/PHET/git/kite/tsconfig-kite-browser.json'

  ../../../../kite/tsconfig-kite-browser.json:4:5
    4     "js/**/*",
          ~~~~~~~~~
    File is matched by include pattern specified here.

error TS6305: Output file 'C:/Users/mjkauzmann/PHET/git/chipper/dist/tsc/outDir/kite/js/kite-tests.d.ts' has not been built from source file 'C:/Users/mjkauzmann/PHET/git/kite/js/kite-tests.js'.
  The file is in the program because:
    Matched by include pattern 'js/**/*' in 'C:/Users/mjkauzmann/PHET/git/kite/tsconfig-kite-browser.json'

  ../../../../kite/tsconfig-kite-browser.json:4:5
    4     "js/**/*",
          ~~~~~~~~~
    File is matched by include pattern specified here.

This article seems to say that it is because we are double counting some resources. I don't know if that is true.

https://stackoverflow.com/questions/78091866/typescript-build-error-ts6305-type-declaration-file-output-file-has-not-been-

Furthermore, I cannot reproduce the same errors consistently. It only happens on --all --clean, and different issues arise each time I run it.

@zepumph zepumph assigned samreid and unassigned zepumph Nov 21, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 21, 2024

@samreid can we pair on this tomorrow?

zepumph added a commit that referenced this issue Nov 22, 2024
@zepumph
Copy link
Member Author

zepumph commented Nov 22, 2024

@samreid and I cannot reproduce now. Our best guess is that it was from me running multiple copies of check at the same time.

@mattpen the upgrade is pushed. Let us know if you have any other questions.

We also noticed that website-meteor has its own dev dependencies for typescript-eslint. Can we remove those and instead use perennial-aliases? That may make this maintenance easier in the future.

Also, website-meteor and website-common each have different versions of Typescript. Perhaps we should use this issue to come together and try to bring our shared dependencies closer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants