Fix flow types #2573
Fix flow types #2573
Conversation
rgbkrk
force-pushed
the
fix-flow-types
branch
2 times, most recently
from
d448a55
to
61de032
Compare
.flowconfig
Outdated
| @@ -4,7 +4,9 @@ types | |||
|
|
|||
| [options] | |||
| suppress_comment= \\(.\\|\n\\)*\\$FlowFixMe | |||
| module.name_mapper='^@nteract/core/(.+)$' -> '<PROJECT_ROOT>/packages/core/src/$1' | |||
| module.name_mapper='^@nteract/core$' -> '<PROJECT_ROOT>/packages/core/src/' | |||
There was a problem hiding this comment.
Any reason for the trailing slash?
There was a problem hiding this comment.
Not really, lemme switch it to make it consistent
There was a problem hiding this comment.
oh, the rest of these down below have a trailing slash so I think I probably kept it consistent there.
.flowconfig
Outdated
| module.name_mapper='^@nteract/core/(.+)$' -> '<PROJECT_ROOT>/packages/core/src/$1' | ||
| module.name_mapper='^@nteract/core$' -> '<PROJECT_ROOT>/packages/core/src/' | ||
| module.name_mapper='^@nteract/core/(?!src)(.+)$' -> '<PROJECT_ROOT>/packages/core/src/$1' | ||
| module.name_mapper='^@nteract/core/src/(.+)$' -> '<PROJECT_ROOT>/packages/core/src/$1' |
There was a problem hiding this comment.
We're going to eventually not allow @nteract/core/* imports, right? Like we're going to change this to be a defacto import from @nteract/core, ya?
There was a problem hiding this comment.
Yeah. Figured I might as well push this to stay consistent for now at least. Once we've switched them, let's take the rule out.
| @@ -2,7 +2,7 @@ jest.mock("fs"); | |||
| import { ActionsObservable } from "redux-observable"; | |||
| import { dummyStore, dummyCommutable } from "@nteract/core/dummy"; | |||
|
|
|||
| import { save, saveAs } from "@nteract/core/actions"; | |||
| import { actions } from "@nteract/core"; | |||
There was a problem hiding this comment.
💯 this is much nicer, imo.
| KERNEL_RAW_STDOUT, | ||
| KERNEL_RAW_STDERR | ||
| } from "@nteract/core/actionTypes"; | ||
|
|
|
|
||
| import { setNotificationSystem } from "@nteract/core/actions"; | ||
| import { actions, providers } from "@nteract/core"; | ||
| const { NotebookApp, Styles } = providers; |
There was a problem hiding this comment.
Nice. So, the only time I've seen this get hairy is when you have a sneaky cyclic import. I.e., it's possible to structure your code such that that providers is actually not yet defined here if there's some cyclic stuff going on, which will set NotebookApp and Styles to undefined for the life of this module. I don't think we're going to run into an issue here, but it's something that I've seen with this pattern before and you just get a runtime error, which can be confusing.
There was a problem hiding this comment.
Otherwise, this is totally fine for now with the hope that we'll actually provide these as a top-level export later.
There was a problem hiding this comment.
Oh I could switch this to the export * from 'providers' setup in @nteract/core and just go from there.
rgbkrk
force-pushed
the
fix-flow-types
branch
from
9fdeb8e
to
26198c5
Compare
Due to a naming conflict, I switched our editor wrapper component to the name <Source>
rgbkrk
force-pushed
the
fix-flow-types
branch
from
26198c5
to
b83e091
Compare
| @@ -196,7 +196,7 @@ export const Example = () => ( | |||
| counter={cell.executionCount} | |||
| /> | |||
| )} | |||
| <Editor> | |||
| <Source> | |||
|
Howdy! I'm 🔓🤖! In order to keep information timely (based on the most recent release), we want all activity to be added to either new issues or open issues and PRs. In service to that goal, I, the lock bot close inactive closed issues when they haven't had activity in 120 days. Feel free to open a new issue for related bugs and link to relevant comments from this thread. |
This addresses some of the shortcomings of our current flow setup with
@nteract/core. Ended up noticing a type mismatch as well, which I fixed by makingmapStateToPropstake a little less for options.