Remove useSingletons, app.singletons, only instantiate kclManager within routeLoaders#10410
Remove useSingletons, app.singletons, only instantiate kclManager within routeLoaders#10410franknoirot wants to merge 8 commits intomainfrom
useSingletons, app.singletons, only instantiate kclManager within routeLoaders#10410Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
There was a problem hiding this comment.
These components and hooks are how I kept the diff on this PR down
| * Until we update these dependents of the settings to take settings | ||
| * as a dependency input, we must subscribe to updates from the outside. | ||
| */ | ||
| onSettingsUpdate = (snapshot: SnapshotFrom<typeof this.settings.actor>) => { |
There was a problem hiding this comment.
This was moved into KclManager here since everything in this handler deals with an executing editor and its subsystems.
There was a problem hiding this comment.
Ding dong useSingletons is dead!
| <ProjectProvider> | ||
| {/** TODO: Decouple opening project from opening an executing editor and move this provider down! */} | ||
| <ExecutingEditorProvider> |
There was a problem hiding this comment.
Here is where I employ those two new components for now. In future PRs, ExecutingEditorProviders will be further down in the component tree, but I need to isolate things a bit more.
| // Cancel all KCL executions while on the home page | ||
| useEffect(() => { | ||
| markOnce('code/didLoadHome') | ||
| kclManager.cancelAllExecutions() |
There was a problem hiding this comment.
Moved into the close method of KclManager
src/lib/routeLoaders.ts
Outdated
| currentFilePath || PROJECT_ENTRYPOINT, | ||
| app.singletons.kclManager, | ||
| // If persistCode in localStorage is present, it'll persist that code | ||
| // through *anything*. INTENDED FOR TESTS. | ||
| window.electron?.process.env.NODE_ENV === 'test' | ||
| ? kclManager.localStoragePersistCode() | ||
| : undefined | ||
| currentFilePath || PROJECT_ENTRYPOINT |
There was a problem hiding this comment.
We no longer provide a KclManager here, we for real instantiate it anew!
1 participant
Marching towards #9956 and #6836. This makes the app not instantiate
kclManageras a permanent "singleton", and in fact removes thebuildSingletonsmethod entirely. This is a refactor that should have no impact on user experience.To keep the diff down and allow React components to continue to treat
projectandexecutingEditoras non-optional, two new components have been introduced:<ProjectProvider />and<ExecutingEditorProvider />, with correspondinguse*hooks. These providers allow use to section off parts of the component tree which shouldn't render if there is no project or executing editor, respectively. In upcoming PRs, I will make these components able to take a fallback component, so that we can provide empty states where needed. Since the router boundary will define whether a project is present or not, that provider won't likely move anytime soon. But there are several parts of the app that need to handle the "no executing editor" state, so that will likely be pushed down the component tree and duplicated in a few places.Some places—notably above the
/fileroute—need access to the current project or executing editor if they are available. For these, I use theuseSignals()hook from Preact, and peel offapp.projectSignal.valueandapp.projectSignal.value?.executingEditor.valuerespectively. There are a few CommandBar input components that require this, as well as Router itself so that the engineCommandManager can be peeled off to power the network health context.