Skip to content

Separate out LoadableState#1

Closed
lsnow99 wants to merge 2 commits intojstrieb:masterfrom
lsnow99:logan/loadable-state
Closed

Separate out LoadableState#1
lsnow99 wants to merge 2 commits intojstrieb:masterfrom
lsnow99:logan/loadable-state

Conversation

@lsnow99
Copy link
Collaborator

@lsnow99 lsnow99 commented Jan 18, 2026

Idea here is to separate out the loadable part of the state since replacing the entire state object leads to elements being overwritten, which should be durable through loads.

The reproduction for the bug is to load a sheet from the Save/Load image modal. Then try pressing i while in normal mode in a cell. It will fail to focus the formula bar with an error like this:

i Edit in Formula Bar index-B_Ckdy7F.js:9154:11
Uncaught TypeError: can't access property "focus", globals2.elements.formulaBar is undefined
    Edit in Formula Bar https://jstrieb.github.io/code-grid/assets/index-B_Ckdy7F.js:8869
    keyboardHandler https://jstrieb.github.io/code-grid/assets/index-B_Ckdy7F.js:9167
    App https://jstrieb.github.io/code-grid/assets/index-B_Ckdy7F.js:9830
    target_handler https://jstrieb.github.io/code-grid/assets/index-B_Ckdy7F.js:2644
    without_reactive_context https://jstrieb.github.io/code-grid/assets/index-B_Ckdy7F.js:1862
    target_handler https://jstrieb.github.io/code-grid/assets/index-B_Ckdy7F.js:2643

}
}

export class State {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff got kind of butchered here since it prioritized my move of the sampleFormulaCode

.then((result) => {
globals = State.load(JSON.parse(result));
globals.load(JSON.parse(result));
globals.imageOpen = false;
Copy link
Collaborator Author

@lsnow99 lsnow99 Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now have to explicitly close the modal since we aren't just relying on the state being wiped. This is probably better to be controlled explicitly imo - you could also handle errors better/show confirmation if you wanted to for example

@lsnow99 lsnow99 changed the title Logan/loadable state Separate out LoadableState Jan 18, 2026
@jstrieb
Copy link
Owner

jstrieb commented Jan 27, 2026

I think this should instead be solvable by fixing how the globals object is stored and passed throughout the codebase. I suspect this bug is a symptom of a fundamentally incorrect way of managing this object. For example, there is double reactivity where the globals object itself is $state, and all its fields are too. This has been useful so far, but feels error prone. I think this bug is proof of that.

All that being said, I'm not sure of the best way to solve the root problem. Possibly with some use of Svelte's setContext and getContext functions.

I'm not going to merge this PR as-is because this requires further thought and investigation.

@lsnow99
Copy link
Collaborator Author

lsnow99 commented Feb 1, 2026

Closing in favor of #2

@lsnow99 lsnow99 closed this Feb 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants