move records into core #2563
move records into core #2563
Conversation
rgbkrk
force-pushed
the
migrate-records
branch
2 times, most recently
from
2f3ed35
to
2768c64
Compare
|
Rebased this against master, still more to go. |
rgbkrk
force-pushed
the
migrate-records
branch
2 times, most recently
from
b152f9b
to
beb6055
Compare
|
This will definitely be rebased... |
|
Stopping this one without doing selectors. I'll do a separate PR for that ball of wax. |
| @@ -4,7 +4,7 @@ import { remote } from "electron"; | |||
| import { from } from "rxjs/observable/from"; | |||
|
|
|||
| import * as nativeWindow from "../../src/notebook/native-window"; | |||
| import { makeAppRecord, makeDocumentRecord } from "@nteract/types/core/records"; | |||
| import { makeAppRecord, makeDocumentRecord } from "@nteract/core/records"; | |||
There was a problem hiding this comment.
As you'll see in most of this PR, it was a lot of turning @nteract/types/core/records into @nteract/core/records
There was a problem hiding this comment.
I've added @nteract/core/types, do you think we should keep /records in that? We're starting to duplicate work as you mentioned, we should make sure we have a plan to resolve the duplication in the future. Thoughts?
| @@ -39,7 +39,9 @@ import * as jmp from "jmp"; | |||
|
|
|||
| import type { NewKernelAction } from "@nteract/core/actionTypes"; | |||
|
|
|||
| import type { KernelInfo, LocalKernelProps } from "@nteract/types/core/records"; | |||
| import type { KernelInfo, LocalKernelProps } from "@nteract/core/src/records"; | |||
There was a problem hiding this comment.
Since the top level packages/core/records doesn't re-export the types, we pull these out of src.
| @@ -26,8 +26,8 @@ import { | |||
| makeAppRecord, | |||
| makeDesktopHostRecord, | |||
| makeDocumentRecord, | |||
| CommsRecord | |||
| } from "@nteract/types/core/records"; | |||
| makeCommsRecord | |||
There was a problem hiding this comment.
Wise change I probably could have done in another PR. It was confusing me as to if this was a type or something else, when it was another record creator.
| @@ -269,6 +269,7 @@ export function exportPDF( | |||
| notificationSystem: * | |||
| ): void { | |||
| const state = store.getState(); | |||
| // $FlowFixMe: This should be using a selector first and foremost | |||
There was a problem hiding this comment.
I'll fix this in a follow up PR when I do the selector bits.
packages/core/src/actionTypes.js
Outdated
| KernelspecsRef, | ||
| KernelspecProps | ||
| } from "@nteract/types/core/records"; | ||
| import type { KernelspecsRef, KernelspecProps } from "./records"; |
There was a problem hiding this comment.
I'll merge this into a later import in this file.
| @@ -142,7 +150,7 @@ export type NotebookMetadata = { | |||
| // orig_nbformat?: number, | |||
| }; | |||
|
|
|||
| export type Notebook = { | |||
| export type NotebookRecordProps = { | |||
There was a problem hiding this comment.
Note: This type isn't used at all here.
| @@ -168,7 +176,7 @@ export const makeAppRecord: RecordFactory<AppRecordProps> = Record({ | |||
| host: null, | |||
| githubToken: null, | |||
| notificationSystem: { | |||
| addNotification: msg => { | |||
| addNotification: (msg: { level?: "error" | "warning" }) => { | |||
There was a problem hiding this comment.
Did this just to increase coverage.
| notebook: ?Notebook, | ||
| // TODO: This _needs_ to become a Record | ||
| notebook: ?Map<string, any>, | ||
| savedNotebook: ?Map<string, any>, |
There was a problem hiding this comment.
We were missing this property on the document record.
rgbkrk
force-pushed
the
migrate-records
branch
from
e017d11
to
fddb435
Compare
...except for in selectors
rgbkrk
force-pushed
the
migrate-records
branch
from
fddb435
to
07283a3
Compare
|
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. |
Closes #2408, moving core records into
@nteract/coreand out of@nteract/types.Necessary follow up PRs:
selectors.$FlowFixMesThere will be some duplication in the middle of this, followed by a magical rebase that makes it look like I did it all in one ✨