Summary:
This diff adds `types` fields on the compiler config for every project. This way we can make sure that for example node types and packages are not available in flipper-ui-core. Without an explicit types field, all types would be shared between all packages, and implicitly included into the compilation of everything. For the same reason `types/index.d.ts` has been removed, we want to be intentional on which types are being used in which package.
This diff does most of the work, the next diff will fine tune the globals, and do some further cleanup.
As an alternative solution I first tried a `nohoist: **/node_modules/types/**` and make sure every package list explicitly the types used in package json, which works but is much more error prone, as for example two different react types versions in two packages will cause the most unreadable compiler error due to the types not being shared and not literally the same.
Reviewed By: lawrencelomax
Differential Revision: D33124441
fbshipit-source-id: c2b9d768f845ac28005d8331ef5fa1066c7e4cd7
Summary: Since no plugin directly uses adbkit or crc32 anymore, we removed them from the globals. They wouldn't work in the browser anyway
Reviewed By: aigoncharov
Differential Revision: D33019084
fbshipit-source-id: 66ab0756399fdb401c63f5e8271bdd62cb79ab4a
Summary:
Forbid imports from nested directories of flipper-common, flipper-plugin, flipper-ui-core
In the stack of D32926830 I occasionally imported from flipper-plugin/src/... which is not allowed. This should fix any auto-import related issues once and for all.
Reviewed By: timur-valiev
Differential Revision: D32987054
fbshipit-source-id: f19f6278219961ad283cacfec094a6c703ca93f8
Summary: Setting the lint level to error and fixing the last three violations.
Reviewed By: nikoant
Differential Revision: D32109025
fbshipit-source-id: 213ecaa17317f665ac5a192682fa06d182c28581
Summary:
Grey -> gray. "Cancelled" seems quite common in APIs though, so I disabled that.
A few promise cleanups
Reviewed By: aigoncharov
Differential Revision: D31323610
fbshipit-source-id: c8863d995936f451c24eb408fe5c26677187f089
Summary:
That's another thing I comment on a lot and is a mostly avoidable source of
bad perf.
Reviewed By: mweststrate
Differential Revision: D30450577
fbshipit-source-id: bb82d8cbd34956fa790243f59cda09ff9c4e7379
Summary:
I hate when I have to comment on a diff and tell somebody to please not use
Her Majesty's Spelling, so I'll let the computer do the shouting.
Mixing two ways of spelling the same thing just isn't fun. I had to
work with an Android library that insisted on spelling it `colour`,
leading to awkward code like `colour: COLORS.BLUE` which is just annoying
and hard to remember.
Reviewed By: lblasa
Differential Revision: D30015807
fbshipit-source-id: 9f913e72617301273dbe12c60b9cdba8cea05537
Summary: Similarly to previous stack, remove the need to import Electron to write things to clipboard. Introduced linter to prevent future use.
Reviewed By: timur-valiev
Differential Revision: D29661777
fbshipit-source-id: 7bc67ede40b65c5f232b69128f3a423e232ddc1b
Summary:
Added rules to ensure we properly propagate errors from promises.
Also found and fixed a place where promise function parameter was mismatched.
Reviewed By: mweststrate
Differential Revision: D28537820
fbshipit-source-id: b93f44274fc76544049813f645508cb78e432880
Summary:
This checks for unnecessary boolean assignments in JSX,
e.g. `grow={true}` which can just be `grow`.
Saw this because mweststrate commented this on a diff.
Reviewed By: mweststrate
Differential Revision: D28329647
fbshipit-source-id: ed408a0d4977315cdd0ff2d13024bb720c27e24a
Summary: It is quite easy to miss some flipper imports, this lint rule makes them more visible. In the future we can eventually turn this into an error.
Reviewed By: jknoxville
Differential Revision: D28027118
fbshipit-source-id: 817b356634cb78b11b9216ce45429ea7a22f0e4f
Summary:
`console.error(err)` are hard to identify in the codebase especially
as we often don't have reliable stack trace information.
I've already cleaned up a bunch of them manually by going after the most
high-firing ones; this should make it easier to identify the remaining ones.
Reviewed By: jknoxville
Differential Revision: D27913964
fbshipit-source-id: 0ff6624a0c083829846550b40954945d655b7cf6
Summary:
For context see https://fb.workplace.com/notes/470523670998369
This diff introduces the DataSource abstraction, that can store records. If a key is set a key -> record mapping is stored, to make it easy to update existing records using `upsert`, without knowing their exact index.
Internal storage will be slightly altered in upcoming diffs, so don't pay to much attention to that part.
Reviewed By: nikoant
Differential Revision: D25953337
fbshipit-source-id: 1c3b53a2fcf61abaf061946be4af21d2aecc6c6d
Summary: When exploratory testing Flipper, I generally see quite some React key warnings. So it seems that plugin devs often miss them. This diff will configure linting more aggressively to address that (it's not fool proof, but will find the most common cases).
Reviewed By: nikoant
Differential Revision: D26722707
fbshipit-source-id: e0d2b56de2422e1147f52c8e9150d00c7ee64bd2
Summary:
I don't think there's an easy way to do this based on types
which would be ideal ...
So instead I'm checking for
- Importing `remote` from `electron`.
- Accessing `electron.remote`.
You can still hack this by importing `electron`, saving
it to a differently named variable and accessing `remote` on it,
but this should cover all reasonable cases we see in real code.
Reviewed By: mweststrate
Differential Revision: D26453006
fbshipit-source-id: 4b3d223bed43ca3f0d1a4f592ea8f8060a823479
Summary: importing `antd` in a plugin that lives in the Flipper repo will give a int warning. This fixes that, antd is provided by the host package.
Reviewed By: nikoant
Differential Revision: D26073161
fbshipit-source-id: 897357fafce20129f7e12c035ff99cb4870cc814
Summary:
* Removed Flow compilation step
* Removed all `flow` annotations
* Removed all FlowFixMe's
* Removed flow typings for Flipper
* Left flow transpilation (stripping) in babel, in case there is any external user using Flow in his plugin
* Left `eslint-plugin-flowtype` dependencies, as `eslint-config-fbjs` requires it
Reviewed By: passy
Differential Revision: D24755545
fbshipit-source-id: 9c0a7910657fd1cba88294e041bf2bfdf7b565bf
Summary:
I noticed that after the typescript upgrade, I got several weird positives from ESLint (like unused parameters in a type definition, which are obviously always unused, e.g. `type onClick = (e: Event) => void`). After some investigation, it turned out these warnings are generated by eslint, but that those rules should be performaned by typescript/eslint instead. For future reference to which rules this applies:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/README.md#extension-rules
Updated the config, and while at it, fixed all warnings in our codebase, except for `react-hooks/exhaustive-deps` warnings, since those require semantic changes.
This reduces the amount of eslint warnings from 86 to 39.
Reviewed By: passy
Differential Revision: D23905630
fbshipit-source-id: 0557708fd9ec6b17840a3c191e7d3baf225bdf23
Summary:
allow-large-files
Updated to typescript 4. Note that this is no new major, the way TS numbers is that 4.0 is simply the number after 3.9 (so they refuse to use 3.10).
Primarily reason is that it allows us to use `/** deprecated */`. although there are definitely more interesting improvements
Reviewed By: passy, nikoant
Differential Revision: D23869445
fbshipit-source-id: 54e251b398b8698d9b18898ce66c3203b32aa356
Summary: Now that the vanilla JS QuickPerformanceLoggerCore was created with the core methods from QPL, we are using this in Flipper so that we have one source of truth
Reviewed By: nubbel
Differential Revision: D23128231
fbshipit-source-id: 16841aab2563509c7184a09ecb8d3f534c53e896
Summary:
Added infra for writing and using custom eslint rules and created a rule for disallowing cross-package file imports. Such imports are anti-pattern and they also break standalone plugin bundling.
We still have a bunch of places where Flipper core references code directly from plugins. I've ignored these places for now and added task T71355623 to revisit them.
Reviewed By: passy
Differential Revision: D22998955
fbshipit-source-id: d04cff8fc115ba1300a7e6830306ec134046e927
Summary:
So far there were 2 types of plugins: `FlipperPlugin` and `FlipperDevicePlugin`. This introduces a third kind: `SandyPluginDefinition`.
Unlike with the old plugins, the export of the module is not directly exposed as the plugin definition. Rather, we use class `SandyPluginDefinition` (instance) that holds a loaded definition and its meta data separately (`PluginDetails`). This means that we don't have to mix in and mutate loaded definitions, and that for unit tests we can avoid needing to provide a bunch of meta data. This also prevents a bunch of meta data existing on two places: on the loaded classes as static fields, and in the meta data field of the loaded class as well. Finally, we can now freely extends the `PluginDetails` interface in flipper, without needing to store it on the loaded classes and we are sure that no naming conflicts are caused by this in the future.
For compatibility with the existing code base, common fields are delegated from the `SandyPluginDefinition` class to the meta data.
Also cleaned up types around plugins a little bit and removed some unnecessary casts.
For all features that reason about plugins in general (such as exports), sandy plugins are ignored for now.
`SandyPluginInstance` is worked out in further diffs
The `instanceof` calls are replaced by a utility function in later diffs.
{F241363645}
Reviewed By: jknoxville
Differential Revision: D22091432
fbshipit-source-id: 3aa6b12fda5925268913779f3c3c9e84494438f8
Summary: Set up an initial library which can (should) be used by plugins in the future.
Reviewed By: jknoxville
Differential Revision: D22019554
fbshipit-source-id: 502b14b34b2c9c117cea377ab6ebbf150e6faee9
Summary:
Had some horrible dev experience caused by VSCode and Jest not agreeing what snapshots should look like, every time saving a test in VSCode, it would break the jest snapshots. After investigating eslint and prettier, turned out VSCode itself was the culprit. Caused by a global user setting, but this makes sure nobody else runs into it :)
Also made sure Jest uses the correct prettier settings (it didn't before).
Reviewed By: passy
Differential Revision: D22114656
fbshipit-source-id: aef6c278a668bce049c96aba95c7a5101ca840ad
Summary:
Enabled linting rules that help to signal making errors with effect dependencies and such.
Fixed all errors, left any warnings generated by the hooks untouched
Reviewed By: nikoant
Differential Revision: D21721497
fbshipit-source-id: 9548453443fa7b663dc4d4289132f388c6697283
Summary: Added eslint rule "no-extraneous-imports" which disallow using modules which are not listed as dependencies in the corresponding package.json. Fixed a bunch of reported errors after the rule applied.
Reviewed By: passy
Differential Revision: D21186848
fbshipit-source-id: 0af9ac4b3fffdfd0ab7c23ae4ff12a3f5989d5e9
Summary:
Quick notes:
- This looks worse than it is. It adds mandatory parentheses to single argument lambdas. Lots of outrage on Twitter about it, personally I'm {emoji:1f937_200d_2642} about it.
- Space before function, e.g. `a = function ()` is now enforced. I like this because both were fine before.
- I added `eslint-config-prettier` to the config because otherwise a ton of rules conflict with eslint itself.
Close https://github.com/facebook/flipper/pull/915
Reviewed By: jknoxville
Differential Revision: D20594929
fbshipit-source-id: ca1c65376b90e009550dd6d1f4e0831d32cbff03
Summary:
Pull Request resolved: https://github.com/facebook/flipper/pull/878
1) Enabled eslint error reporting for unresolved imports and fixed all the errors
2) Enabled eslint for typings (d.ts) and fixed all the errors
Reviewed By: passy
Differential Revision: D20335151
fbshipit-source-id: 7b142281a406b32df0f02a5cd0d7d05eba941acd
Summary:
Pull Request resolved: https://github.com/facebook/flipper/pull/872
Move all the JS code related to desktop app to "desktop" subfolder.
The structure of "desktop" folder:
- `src` - JS code of Flipper desktop app executing in Electron Renderer (Chrome) process. This folder also contains all the Flipper plugins in subfolder "src/plugins".
- `static` - JS code of Flipper desktop app bootstrapping executing in Electron Main (Node.js) process
- `pkg` - Flipper packaging lib and CLI tool
- `doctor` - Flipper diagnostics lib and CLI tool
- `scripts` - Build scripts for Flipper desktop app
- `headless` - Headless version of Flipper app
- `headless-tests` - Integration tests running agains Flipper headless version
Reviewed By: passy
Differential Revision: D20249304
fbshipit-source-id: 9a51c63b51b92b758a02fc8ebf7d3d116770efe9
