V7: Migrate all lint tooling to ESLint #644
Conversation
bengourley
changed the title
TSLint is deprecated. ESlint was already being used under the hood by standard JS. This updates the entire linting setup so that common ESLint dependecies are used for both JS and TS lint, just with different presets.
TSlint was removed via the last commit, a clean and rebootstrap removes this from the package-lock.json files.
bengourley
force-pushed
the
v7-lint-tooling
branch
from
525058c
to
4f48fa5
Compare
| @@ -102,7 +102,7 @@ module.exports = { | |||
| 'Notification', 'SVGElementInstance', 'Screen', 'TextTrack', 'TextTrackCue', 'TextTrackList', | |||
| 'WebSocket', 'WebSocketWorker', 'Worker', 'XMLHttpRequest', 'XMLHttpRequestEventTarget', 'XMLHttpRequestUpload' | |||
| ], o => { | |||
| if (!win[o] || !win[o].prototype || !win[o].prototype.hasOwnProperty || !win[o].prototype.hasOwnProperty('addEventListener')) return | |||
| if (!win[o] || !win[o].prototype || !Object.prototype.hasOwnProperty.call(win[o].prototype, 'addEventListener')) return | |||
There was a problem hiding this comment.
A quick search suggests Object.prototype.hasOwnProperty.call is supported and preferred whereas hasOwnProperty doesn't exist for "host" objects such as window in IE8. Possibly a slight change in behaviour to be aware of.
There was a problem hiding this comment.
Yeah, I presume this was the reason the linter told me change it!
| @@ -2,21 +2,21 @@ import * as BugsnagCore from "@bugsnag/core"; | |||
|
|
|||
| // overwrite config interface, adding browser-specific options | |||
| declare module "@bugsnag/core" { | |||
| interface IConfig { | |||
There was a problem hiding this comment.
I guess these changes to the external type names constitute a breaking change.
There was a problem hiding this comment.
👍 yup this will be shipped in a major release with a whole bunch more breaking changes. Stay tuned!
There was a problem hiding this comment.
@bengourley @djskinner should we have an UPGRADING.md or whatever as part of this change that highlights that this is required? Should this be in the CHANGELOG as part of this PR?
There was a problem hiding this comment.
This PR is already merged but I'm happy to include changelog entries for subsequent PRs. Upgrading guide-wise I think that should be done once everything has landed on the `v71 integration branch.
TSLint is deprecated. ESLint was already being used under the hood by standard JS. This PR updates the entire linting setup so that common ESLint dependecies are used for both JS and TS lint, just with different parsers/presets.
The changeset in this PR can be summarised as follows:
Add and remove tooling
tslinteverywhere it was includedstandardeslintas a direct dependency and the modules required to get an equivalent-ish standardjs setup@typescript/eslint-*namespacetslint.json(multiple locations).eslintrc.jsand.eslintrs.ts.jswhich set up different rules for each kind of source filepackage.jsonRun
eslint --fixon existing codeAlthough we used the same "standard" preset, it hadn't been updated in a while so there were some violations sprinkled around – mostly relating to whitespace or unnecessary quoting of object keys.
Manually fix remaining lint errors
A few JS violations had to be resolved by hand – mostly telling ESLint to ignore the next line for things that needed to be "just so".
Since the TypeScript rules changed quite a lot from TSLint to
@typescript/eslint-*, a few tweaks were required, including removing theI*prefix from interface names.This PR is split off from work that was done in the
spec-refactorbranch.