CT-518 add charm on navigateTo in jumble #1343
Conversation
we were creating the charm cells improperly (a second charm would share cells with the first due to cause naming) - additionally navigate shouldn't navigate if it returns undefined
| return this.charms; | ||
| } | ||
|
|
||
| private async add(newCharms: Cell<Charm>[]) { |
There was a problem hiding this comment.
@jsantell given your (upcoming) work on internal charm apis/functions, this should probably be considered
an action in jumble can mean "actually, you know this charm, plz remember it for me!)
There was a problem hiding this comment.
cubic found 2 issues across 6 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| } | ||
|
|
||
| runtime.navigateCallback(target); | ||
| if (target && target.get()) { |
There was a problem hiding this comment.
The check target.get() assumes that target is an object with a get method, but there is no type guard to ensure target has a get function. This could throw a runtime error if target is not the expected type.
| if (target && target.get()) { | |
| if (target && typeof target.get === 'function' && target.get()) { |
|
fyi @bfollington not sure if you saw weirdness with compileAndRun. I was hitting errors trying to navigate to the second charm I generated due to issues with navigate & sharing cells... |
Summary by cubic
Added logic to ensure charms are added to the charm manager when navigating to them in Jumble, and fixed an issue where navigating to a charm could fail or reuse cells incorrectly.