Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement resolveIdentifier on readonly trees #79

Merged
merged 1 commit into from
Feb 16, 2024
Merged

Conversation

airhorns
Copy link
Contributor

@airhorns airhorns commented Jan 22, 2024

Got a not yet implemented gadget side when going to use this API, figured it wasn't too hard! We implement by keeping the identifier cache around beyond just after we instantiate, instead of only capturing a reference to the env from that context.

@airhorns airhorns changed the title Implement resolveIdentifier on readonly trees Implement resolveIdentifier on readonly trees Jan 22, 2024
@airhorns airhorns marked this pull request as ready for review January 22, 2024 00:07
@airhorns airhorns requested a review from thegedge January 22, 2024 00:07
Copy link
Contributor

@thegedge thegedge left a comment

Choose a reason for hiding this comment

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

Nice! 🚀

src/api.ts Outdated
@@ -150,23 +151,32 @@ export function getType(value: IAnyStateTreeNode): MSTAnyComplexType | IAnyCompl
return value[$type];
}

export function getEnv<Env = any>(value: IAnyStateTreeNode): Env {
/** @hidden */
export function getContext(value: IAnyStateTreeNode): InstantiateContext | null {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: since it's used beyond instantiation now, I'd be down for giving InstantiateContext a more generic name.

spec/api.spec.ts Outdated
expect(resolveIdentifier(NamedThingClass, instance.map.get("test_key")!, "test_key")?.key).toEqual("test_key");
});

test("returns null when the identifier doesn't resolve", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

super nit: returns undefined (which is good too, because that's what MST does!)

@airhorns airhorns force-pushed the reference-support branch 2 times, most recently from e5569e7 to ea7d0e1 Compare February 16, 2024 16:13
@airhorns airhorns merged commit 81969e4 into main Feb 16, 2024
5 checks passed
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