search

ember-cli / eslint-plugin-ember

An ESLint plugin that provides set of rules for Ember applications based on commonly known good practices.
MIT License
260 stars 201 forks source link

Warning for Gjs/Gts setup instructions always returned - even with correct setup #2160

Open IgnaceMaes opened 3 weeks ago

IgnaceMaes commented 3 weeks ago

When doing a fresh setup following the instructions in the README, I'm seeing the following behaviour.

New .gjs file

empty:

/.../app/components/example.gjs
  0:0  warning  
To lint Gjs/Gts files please follow the setup guide at https://github.com/ember-cli/eslint-plugin-ember#gtsgjs

when adding an example lint violation, e.g.:

let x = 1;
<template>{{x}}</template>

Running eslint now gives:

/.../app/components/test.gjs
  2:13  error  update-able variables are not supported in templates, reference a const variable
To lint Gjs/Gts files please follow the setup guide at https://github.com/ember-cli/eslint-plugin-ember#gtsgjs  ember/template-no-let-reference

Notice how the warning now gets included with the first actual issue.

patricklx commented 3 weeks ago

Can you check if there are multiple instances of ember-eslint-parser in your node-modules? That could cause the issue you are seeing

IgnaceMaes commented 3 weeks ago

Oh I see where this is going wrong.

In my lockfile I have two versions of ember-eslint-parser:

As it's a pre-1.0 release the version resolution with ^ doesn't merge them.

Adding a dependency resolution override fixes the problem:

  "resolutions": {
    "ember-eslint-parser": "^0.5.0"
  },

Thanks for the hint!

I suppose we should bump the dependency here. Or to prevent future issues, look into cutting a 1.0 release of ember-eslint-parser?

NullVoxPopuli commented 3 weeks ago

why have resolutions at all?

IgnaceMaes commented 2 weeks ago

why have resolutions at all?

Can you elaborate? Not sure if I'm following here.

Without the resolution it pulls in two copies of ember-eslint-parser which triggers the warning.

NullVoxPopuli commented 2 weeks ago

Without the resolution it pulls in two copies of ember-eslint-parser which triggers the warning.

ah, let's fix that :sweat_smile:

IgnaceMaes commented 2 weeks ago

It might just not be recommended to install ember-eslint-parser as a direct dependency in the consuming app? Not sure how I got to that, as it's not in the README guides of this repo.

Though that does mean the version is thighly couples to this package, and because of pre-1.0 semver resolutions it doesn't resolve to the latest available version.

NullVoxPopuli commented 2 weeks ago

It might just not be recommended to install ember-eslint-parser as a direct dependency in the consuming app?

correct, no one should do this :sweat_smile:

because of pre-1.0 semver resolutions it doesn't resolve to the latest available version.

yeah -- given that we hadn't had any bug reports in a while, I wonder if it's time to bump the parser to 1.0