Skip to content

feat: add observable naming, creation hook, and babel auto-naming transform#645

Open
DorianMazur wants to merge 1 commit intomainfrom
feat/devtools-observable-naming
Open

feat: add observable naming, creation hook, and babel auto-naming transform#645
DorianMazur wants to merge 1 commit intomainfrom
feat/devtools-observable-naming

Conversation

@DorianMazur
Copy link
Collaborator

@DorianMazur DorianMazur commented Mar 11, 2026

Add devtools infrastructure: observable naming + creation tracking

This adds the core pieces needed to build a devtools plugin (e.g. for rozenite).

What changed:

  • observable() and observablePrimitive() now accept an optional second arg { name: 'myStore' } which sets _name on the root node
  • New onObservableCreated(handler) global hook in middleware.ts, fires synchronously when any observable is created, zero cost when nothing is subscribed
  • Babel plugin extended to auto-inject names: const foo = observable({}) becomes const foo = observable({}, { name: 'foo' }) at compile time
  • _name field added to BaseNodeInfo, child path is derivable by walking parent/key chain

Why:

To build a legend-state devtools plugin, you need to be able to discover observables as they're created and show meaningful names for them. Without compiler support, observables are anonymous. The babel transform solves this automatically.

@DorianMazur DorianMazur requested a review from jmeistrich March 11, 2026 15:28
@DorianMazur DorianMazur changed the title feat: add observable naming, creation hook, and babel plugin feat: add observable naming, creation hook, and babel auto-naming transform Mar 11, 2026
@DorianMazur DorianMazur requested a review from Copilot March 12, 2026 17:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds devtools-facing infrastructure to improve discoverability of observables at runtime and provide stable names for debugging, including compiler assistance via the Babel transform.

Changes:

  • Added optional { name } options to observable / observablePrimitive, storing _name on the root node.
  • Added a global onObservableCreated hook (and internal notifier) fired synchronously on observable creation.
  • Extended the Babel plugin to auto-inject { name: '<varName>' } into observable factory calls, plus tests for naming + creation hooks.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/devtools.test.ts New tests covering _name, creation hook behavior, and path derivation via parent/key chain.
tests/babel.test.ts New Babel plugin tests verifying auto-name injection behavior and non-target cases.
src/observableInterfaces.ts Adds ObservableOptions and _name?: string to node info for naming support.
src/observable.ts Threads optional ObservableOptions into observable factory functions.
src/middleware.ts Adds global onObservableCreated subscription + notification implementation.
src/createObservable.ts Sets _name from options and calls notifyObservableCreated after creation.
src/babel/index.ts Implements auto-name injection for observable / observablePrimitive based on import + variable name.
index.ts Exposes onObservableCreated via internal export for downstream integrations.
Comments suppressed due to low confidence (1)

src/babel/index.ts:33

  • In the @legendapp/state/react import scan, this loop assumes every specifier is an ImportSpecifier (accesses specifiers[i].imported.name). Default or namespace imports would make imported undefined and can crash the Babel plugin. Please guard on spec.type === 'ImportSpecifier' before reading spec.imported.name (similar to the observable import handling below).
                    if (source === '@legendapp/state/react') {
                        const specifiers = path.node.specifiers;
                        for (let i = 0; i < specifiers.length; i++) {
                            const s = specifiers[i].imported.name;
                            if (!hasLegendImport && (s === 'Computed' || s === 'Memo' || s === 'Show')) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +77 to +80
if (args.length === 0) {
// observable() -> observable(undefined, { name: '...' })
args.push(identifier('undefined'));
args.push(nameOption);
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

For the zero-arg call case, the transform injects an Identifier named undefined. Since undefined can be shadowed by a local binding, this can change runtime semantics in some files. Prefer generating an unshadowable undefined value (e.g., void 0) instead of identifier('undefined').

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +35
for (const handler of creationHandlers) {
try {
handler(node);
} catch (error) {
console.error('Error in onObservableCreated handler:', error);
}
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

notifyObservableCreated iterates directly over creationHandlers. If handlers are added/removed during notification (including a handler unsubscribing itself), iteration order/coverage can become surprising. Consider iterating over a snapshot (e.g., Array.from(creationHandlers)) similar to how middleware events snapshot handler sets later in this file.

Copilot uses AI. Check for mistakes.
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