feat: get @signal, @memo, and @effect decorators working with #private members
#15
Conversation
…private members
feat: also improve and test additional inheritance scenarios, adding a new option parameter for `@signal` to disable default extension behavior, defaulting to true: `@signal({ extend: false })`. When `extend` is set to `false`, a subclass signal will *override* the parent class signal instead of re-using it, but that will cause any base class effects depending on the base class signal to no longer re-run when the subclass signal changes.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive support for JavaScript #private members with the @signal, @memo, and @effect decorators. It also introduces a new inheritance mechanism with an extend option for @signal that defaults to true, allowing subclasses to override signal initial values while keeping base class effects and memos operational.
Key changes include:
- Private field detection and appropriate error handling for
@signalon private fields - Full support for private getters/setters and auto accessors across all decorators
- New
SignalOptionsinterface withextendproperty for controlling inheritance behavior - Enhanced metadata structure with context tracking and signal reuse capabilities
Reviewed changes
Copilot reviewed 10 out of 37 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/signals/signalify.ts | Adds Undefined symbol and optional initialVal parameter to support distinguishing undefined from unset values |
| src/decorators/types.ts | Introduces MemberType (renamed from SignalOrMemoType), adds new metadata fields for signal reuse and private memo storage |
| src/decorators/signal.ts | Major refactor with overloaded signatures, private member support, inheritance handling via extend option, and signal descriptor reuse logic |
| src/decorators/signal.test.ts | Comprehensive test coverage for private members and inheritance scenarios |
| src/decorators/memo.ts | Implements private member support for getters/setters, auto accessors, and methods using separate code paths |
| src/decorators/memo.test.ts | Tests for private memo members across all supported kinds |
| src/decorators/effect.ts | Updated to use new API signatures and support private members through context.access |
| src/decorators/effect.test.ts | Extended tests for private method and auto accessor effects |
| src/_state.ts | Refactored helper functions to accept MemberStat instead of individual parameters, adds untrack import |
| dist/* | Generated distribution files reflecting all source changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // CONTINUE writing not working yet for private method memos | ||
| // Uncomment when fixed | ||
| debugger |
There was a problem hiding this comment.
There's a debugger statement left in test code that should be removed before merging.
| /** | ||
| * | ||
| */ |
There was a problem hiding this comment.
The comment on line 36 is empty. Either add meaningful documentation or remove this empty JSDoc comment block.
| memoFn.length === 0 | ||
| ? [createMemo(() => memoFn.call(self)), (() => {}) as Setter<any>] | ||
| : createWritableMemo(() => { | ||
| debugger |
There was a problem hiding this comment.
There's a debugger statement left in the code. This should be removed before merging to production.
feat: also improve and test additional inheritance scenarios, adding a new option parameter for
@signalto disable default extension behavior, defaulting to true:@signal({ extend: false }). Whenextendis set tofalse, a subclass signal will override the parent class signal instead of re-using it, but that will cause any base class effects depending on the base class signal to no longer re-run when the subclass signal changes.