Linting fixes (main process & scripts)

[goosewobbler]

Addressing all Main process and script file linting issues.

Note we are not addressing warnings at the moment - there are a lot of any types which we absolutely should replace as our use of TS in Frame expands and improves, but addressing this now requires a fair bit of work and also high levels of risk due to the amount of refactoring involved.

[mholtzman]

is there a way to define globals, ahem, globally?

[goosewobbler]

There is, we're using the globals package for this.

https://github.com/floating/frame/pull/1346/files#diff-9601a8f6c734c2001be34a2361f76946d19a39a709b5e8c624a2a5a0aade05f2R55
https://github.com/sindresorhus/globals/blob/main/globals.json

However I thought this file is renderer process only -- 'global' (as opposed to 'globalThis') and 'process' are only available in the node environment, we are only setting globals.browser (see link above). I logged them out and verified that they do exist at runtime, though. Basically I got confused and decided to exempt them here...

EDIT:
Seems 'global' and 'process' are both available in both renderer and main processes. I think the electron docs are a bit hazy on this, couldn't find any info on 'global' and it's really not obvious where 'process' is available and what you have access to. I tried changing the use of 'process' to the imported object from electron and it broke everything. globals doesn't have any config for electron, but I added these objects to our eslint config and removed the /* globals */ comments.

https://www.geeksforgeeks.org/process-object-in-electronjs/
https://iamveryhungry.medium.com/share-data-using-global-d141cfc21b7d

[mholtzman]

do we have a lot of functions that require this carve out? would prefer to keep it on if possible

[goosewobbler]

Current totals in this branch at time of typing:
Rule on: 254 problems (173 errors, 81 warnings)
Rule off: 248 problems (167 errors, 81 warnings)

So 6 instances. I would rather keep it on too, could have a single exemption in a resources file and any file where we're using an empty func imports that? I'm also trying to reduce refactoring before the omni release.
e.g. import { noop } from 'resources/utils'

[goosewobbler]

We can still make the above change and disable it completely (with one exemption as mentioned above) but as an intermediary I restricted the rule to arrow functions only.

[mholtzman]

are there cases where we want to keep the _var format for specific vars? I'd prefer to use the after-used option here personally: https://eslint.org/docs/latest/rules/no-unused-vars#args

[goosewobbler]

Personally I'm not a fan of comma dangle outside of this use case as it's usually just a crutch to avoid having to name things properly or even more lazily, to avoid having to restrict access to methods / properties which should be private. Note that I didn't specify varsIgnorePattern here. Updated to use after-used

[mholtzman]

for my own information, what rule causes us to remove these React import statements from lots of files, why aren't they needed, and were they at some point in the past?

[goosewobbler]

These were previously required because JSX required a React import to be in scope in order to use it. React 17 changed that with the new JSX transform.

https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html#removing-unused-react-imports
https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/react-in-jsx-scope.md

[mholtzman]

@goosewobbler looks like some tests are broken

[goosewobbler]

@mholtzman there were shenanigans with ledger tests breaking on some intra lib import. I think it was some weird npm / lockfile behaviour. Should be fixed now, otherwise will need to recreate with cherrypick

[goosewobbler]

different error, another odd one. Will address this and the rest of the comments tomorrow / monday

[goosewobbler]

Remaining errors to address:

Don't use {} as a type. {} actually means "any non-nullish value".

• If you want a type meaning "any object", you probably want object instead.
• If you want a type meaning "any value", you probably want unknown instead.
• If you want a type meaning "empty object", you probably want Record<string, never> instead.
  @typescript-eslint/ban-types

frame/main/transaction/actions/index.ts

Line 11 in a8bdc8e

update?: (request: AccountRequest, params: Partial<T>) => {}

frame/main/externalData/coingecko/index.ts

Line 25 in a8bdc8e

platforms?: {}

Do not use "@ts-ignore" because it alters compilation errors.
@typescript-eslint/ban-ts-comment

frame/main/nebula/index.ts

Line 5 in a8bdc8e

// @ts-ignore

frame/main/nebula/index.ts

Line 10 in a8bdc8e

// @ts-ignore

Promise executor functions should not be async.
no-async-promise-executor:

frame/main/externalData/inventory/tokens.ts

Line 48 in a8bdc8e

return new Promise<TokenSpec[]>(async (resolve, reject) => {

Don't use Function as a type. The Function type accepts any function-like value.
It provides no type safety when calling the function, which can be a common source of bugs.
It also accepts things like class declarations, which will throw at runtime as they will not be called with new.
If you are expecting the function to accept certain arguments, you should explicitly define the function shape. @typescript-eslint/ban-types

frame/main/externalData/balances/index.ts

Line 44 in a8bdc8e

let onResume: Function | null

'task' is not defined.
no-undef

frame/hardhat.config.js

Line 5 in a8bdc8e

return task(taskName, taskDescription)

[goosewobbler]

Closing in favour of #1470