Change AbortController to default import #579
Conversation
| @@ -1,7 +1,7 @@ | |||
| import contractMap from '@metamask/contract-metadata'; | |||
| import type { Patch } from 'immer'; | |||
| import { Mutex } from 'async-mutex'; | |||
| import { AbortController } from 'abort-controller'; | |||
| import AbortController from 'abort-controller'; | |||
There was a problem hiding this comment.
Let's use a // eslint-disable-next-line directive to avoid modifying the ESLint config
There was a problem hiding this comment.
Ok. Can do. Just wondering though is that a lint rule we want to preserve? It feels like we may have opted into it by default?
There was a problem hiding this comment.
Yeah we probably did, but the defaults are generally reasonable, and as a matter of expediency discussions about which lint rules we should use should be had on our ESLint config repo. For the purposes of this PR, I think it's best if we only concern ourselves about how we disable this particular rule for this particular instance.
There was a problem hiding this comment.
Fair! Will update!
There was a problem hiding this comment.
Although, now that we're here, I think that rule is good, because consider a module with both named and default exports. Many such packages exist, and the named and default exports are usually different. If you import the default export under the name of a named export, you find yourself in an annoying place if you ever also want to use the named export, since one of the names have to change. That seems bad to me!
This polyfill package is an exception in that the default and named export are the same thing, and IMO we should call them both AbortController.
There was a problem hiding this comment.
Makes sense. I've swapped out the config change for a eslint-disable-next-line
adonesky1
force-pushed
the
default-import-abortcontroller
branch
from
1cc9eb2
to
f4ee38d
Compare
adonesky1
force-pushed
the
default-import-abortcontroller
branch
from
f4ee38d
to
659d374
Compare
* Change AbortController to default import * Update src/assets/TokenListController.ts Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
* Change AbortController to default import * Update src/assets/TokenListController.ts Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
🤦
FIXED:
AbortController(polyfill) constructor as a named import. Despite the package seeming to support named imports, this change seems to be required for the TokenListController to not blow up.