-
Notifications
You must be signed in to change notification settings - Fork 251
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
V7: Migrate all lint tooling to ESLint #644
Conversation
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these changes to the external type names constitute a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
tslint
everywhere it was includedstandard
eslint
as a direct dependency and the modules required to get an equivalent-ish standardjs setup@typescript/eslint-*
namespacetslint.json
(multiple locations).eslintrc.js
and.eslintrs.ts.js
which set up different rules for each kind of source filepackage.json
Run
eslint --fix
on 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-refactor
branch.