Move gjs/gts parser to ember-eslint-parser library
#2028
Conversation
NullVoxPopuli
force-pushed
the
move-parser-code-to-library
branch
3 times, most recently
from
3729df5
to
98cbe37
Compare
There was a problem hiding this comment.
Since ember-eslint-parser now specifies dependencies we need:
Can we move these dependencies back to devDependencies:
@babel/eslint-parser
And remove these dependencies:
@babel/core@typescript-eslint/parser@typescript-eslint/scope-manager@typescript-eslint/scope-manager
Can we also remove the TypeScript peerDependencies/peerDependenciesMeta?
There was a problem hiding this comment.
@babel/eslint-parser- yeah, looks like its only referenced in tests an dev config@babel/core-- yep, nothing references it@tyepscript-eslint/parser- nope, it's used in tests, and specified ineslint-remote-tester.config.js-- if that file is a dev-only dependency, we can move to devDeps tho@typescript-eslint/scope-manager-- yup, nothing references it
There was a problem hiding this comment.
eslint-remote-tester.config.js is only for testing, so yes let's move @tyepscript-eslint/parser to dev deps.
There was a problem hiding this comment.
excellent, thanks!
There was a problem hiding this comment.
Can we also remove the TypeScript peerDependencies/peerDependenciesMeta?
There was a problem hiding this comment.
we have to keep typescript as a peer, because ember-eslint-parser declares it as a peer.
yarn won't complain, but pnpm would raise an eyebrow at you ✨
When a dep declares a peer, we as a middle-library, must either:
- forward the peer
- provide the dependency
We don't want to actually provide the dependency, and ember-eslint-parser doesn't want us to either.
We want the end-consumer's typescript copy to be the one that's used.
appease the linter for now More lints Not enough lint:fix Use published version bump version eh ohe
NullVoxPopuli
force-pushed
the
move-parser-code-to-library
branch
from
01f8dac
to
33cac2d
Compare
There was a problem hiding this comment.
- Does this file include sufficient integration tests to ensure the parser works with eslint-plugin-ember?
- Can tests be added inside https://github.com/NullVoxPopuli/ember-eslint-parser too?
There was a problem hiding this comment.
-
yeah, due to:
- this config: https://github.com/ember-cli/eslint-plugin-ember/pull/2028/files/47ba1338ffb48f82b50bc44a2d4630f61f85504b#diff-25f04c9ea7361cc9ea80710350c44fe78b386a31e58349a86e3c819ce143149fL820
- and these files in the
ember_tsfolder: https://github.com/ember-cli/eslint-plugin-ember/blob/master/tests/lib/rules-preprocessor/ember_ts/foo.gts
-
yeah, that's planned
There was a problem hiding this comment.
Okay, I think we need tests in ember-eslint-parser ASAP, before v12 final, as that library could be at risk of breaking eslint-plugin-ember users if a bad/untested change gets into it.
There was a problem hiding this comment.
absolutely -- @patricklx do you happened to have time for that?
There was a problem hiding this comment.
I might have a bit of time until beginning of January, afterwards only after end of January
There was a problem hiding this comment.
I think it should be possible to just copy the Tests. We do not need to test any ember specific rules there. Just use pure eslint as dev dependency
bmish
changed the title
ember-eslint-parser library
|
Can you add some context about why this code is/should moving into a separate library in the PR description? |
done! |
|
Making sure no changes in ember-eslint-parser can break eslint-plugin-ember: ember-tooling/ember-eslint-parser#13 |
Collab with @patricklx and @vstefanovic97
Parser: https://github.com/NullVoxPopuli/ember-eslint-parser/
Why move the parser to a separate library?
content-tagTodo:
These existing tests currently fail (as do a few other things that use gjs/gts parsing)
GOAL -- all tests should become passing without changes
Related PRs that explored and introduced the parser:
<template>blocks (not allowed) #2005