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

@typescript-eslint breaks after update to eslint-plugin-ember v12 #2062

Closed
boris-petrov opened this issue Jan 15, 2024 · 19 comments · Fixed by ember-tooling/ember-eslint-parser#49
Labels

Comments

@boris-petrov
Copy link

With version 11.12.0 (and everything else at the latest version) all works fine. Upgrading to v12 I get 400+ errors from ESLint. As far as I see, all are from @typescript-eslint. The vast majority are 'any' overrides all other types in this union type @typescript-eslint/no-redundant-type-constituents (not sure if that matters).

Anyone else hit this problem? Any ideas what might be happening?

@NullVoxPopuli
Copy link
Contributor

in V11, type-aware lint rules were not able to run on gts files, now they are -- are you seeing something outside of the expected behavior from @typescript-eslint/no-redundant-type-constituents?

@boris-petrov
Copy link
Author

I should have mentioned that I'm not using GTS files yet, still on the "old" Ember style (but on the latest version).

Yes, all of these new errors that I get are bogus - they don't make sense. At least I think so.

@NullVoxPopuli
Copy link
Contributor

Any chance you could create a reproduction repo?
I'm curious why, if you aren't using gts, why typescript-eslint rules are active at all?

Can you provide your config?

@boris-petrov
Copy link
Author

I'm using TS, why wouldn't they be active? I'm just not using GTS.

My config is pretty big. If you can't think of anything on the top of your head what might be happening, I'll try creating a reproduction shortly.

@NullVoxPopuli
Copy link
Contributor

Are you importing from gjs? perhaps?

but yeah, a reproduction would be most helpful, as I don't have an ideas for anything that would cause the behavior you're seeing

@boris-petrov
Copy link
Author

No, I don't have any GTS in my code (and I guess I don't have in my plugins either?).

@bmish
Copy link
Member

bmish commented Jan 15, 2024

Our new dependency on ember-eslint-parser could bring in some newer TypeScript dependencies. Could that be related?

@bmish bmish added the bug label Jan 15, 2024
@boris-petrov
Copy link
Author

@bmish it could be, however, I'm on the newest version of everything so... not sure.

Here is a repo. Clone and run pnpm lint:js. You'll see the error. Funny thing is that downgrading doesn't seem to fix it... it does on my project. And the minified version of my project that I was trying while reproducing...

If anyone has ideas... for me there is something very strange going on.

@boris-petrov
Copy link
Author

No, wait. This error seems to appear if the import is not found. If you change ../store to asdf/services/store it will fail in that way. In my project the Ember app is in a folder called frontend and all my imports are like frontend/services/store. Could this have somehow changed?

@NullVoxPopuli
Copy link
Contributor

it's possible your tsconfig resolution settings aren't expecting extensionless imports?

I'll dive in to your repro tomorrow

@boris-petrov
Copy link
Author

I believe my tsconfig should be fine but I can't be sure. In any case, please check out the reproduction. Run pnpm i and then pnpm lint:js. You'll see the error. Downgrading to 11.12.0 fixes the issue.

@chancancode
Copy link

I have also attempted to upgrade and ran into the same issue. I attempted to make sure everything @typescript-eslint is already up-to-date before the eslint-plugin-ember upgrade itself to isolate the issue as much as possible.

Here is the upgrade diff: https://gist.github.com/chancancode/966bf0ff4df59c919839113f368b1a7a

I also tried deleting the eslint cache (before/after the upgrade) just to be sure, but doesn't seem to be the case either.

It definitely seems like something about this upgrade broke something in the module graph, either things are not resolving, or some module isn't able to be parsed correctly, and causing that to cascade outwards.

It's causing non-ember related errors like these:

Screen Shot 2024-02-12 at 7 50 30 PM

Here, this.modelFor() has a return type of unknown so the only reason eslint complains about any would be because the AppComponent import failed to resolve and became any.

I have a pretty strong hunch that the problem is we somehow broke support for import type, and that they are silently getting ignored (the errors are as if you comment out all the import type lines).

@bmish
Copy link
Member

bmish commented Feb 13, 2024

@boris-petrov
Copy link
Author

Thank you for working on this!

However, the new version (12.0.1) doesn't seem to fix either the problem in my repo, or the reproduction I gave above. Can someone check?

@NullVoxPopuli
Copy link
Contributor

your lockfile needs refreshed -- it's using a very old parser

@boris-petrov
Copy link
Author

@NullVoxPopuli it's not that. I pushed another commit that updates everything. Still doesn't work.

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Feb 13, 2024

your lockfile is still using too old of a parser: https://github.com/boris-petrov/eslint-plugin-ember-bug/blob/master/pnpm-lock.yaml#L790

should be 0.3.4.

make sure pnpm is > 8.7 so you get resolution-mode=highest set by default.

if that is the case, then we probably need to force a bump in eslint-plugin-ember.
.. we should probably do that anyway, really

@boris-petrov
Copy link
Author

It's a bug in eslint-plugin-ember. This doesn't allow 0.3.x.

@NullVoxPopuli
Copy link
Contributor

PR to update here: #2091

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

Successfully merging a pull request may close this issue.

4 participants