You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I'd like to suggest a small ergonomic improvement to Signal.prototype.subscribe.
Currently, subscribe calls fn(value) with no this context. This means you can cause a ReferenceError when the condition is already true on the first (immediate) call:
constvalueSignal=signal(true);constunsubscribe=valueSignal.subscribe((value)=>{if(value===true){unsubscribe();// ReferenceError — unsubscribe not assigned yet}});
This is actually a sign my code should be checking before subscribing in the first place:
But with effect() I'd expect to write this more naturally — yet it doesn't work either today. I noticed the TypeScript signature already anticipates it:
But the implementation doesn't deliver on it because _callback calls this._fn() without binding.
Proposal: Two small changes:
In Effect.prototype._callback, call this._fn.call(this) instead of this._fn():
Effect.prototype._callback=function(){constfinish=this._start();try{if(this._flags&DISPOSED)return;if(this._fn===undefined)return;constcleanup=this._fn.call(this);// bind Effect as `this`if(typeofcleanup==="function"){this._cleanup=cleanup;}}finally{finish();}};
In Signal.prototype.subscribe, use a regular function and capture the signal reference explicitly so this refers to the Effect:
Signal.prototype.subscribe=function(fn){constsignal=this;returneffect(function(){constvalue=signal.value;constprevContext=evalContext;evalContext=undefined;try{fn.call(this,value);// this = Effect instance}finally{evalContext=prevContext;}},{name: "sub"});};
This would allow both effect and subscribe to self-dispose cleanly:
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
Hi,
I'd like to suggest a small ergonomic improvement to
Signal.prototype.subscribe.Currently,
subscribecallsfn(value)with nothiscontext. This means you can cause aReferenceErrorwhen the condition is already true on the first (immediate) call:This is actually a sign my code should be checking before subscribing in the first place:
But with
effect()I'd expect to write this more naturally — yet it doesn't work either today. I noticed the TypeScript signature already anticipates it:But the implementation doesn't deliver on it because
_callbackcallsthis._fn()without binding.Proposal: Two small changes:
Effect.prototype._callback, callthis._fn.call(this)instead ofthis._fn():Signal.prototype.subscribe, use a regular function and capture the signal reference explicitly sothisrefers to the Effect:This would allow both
effectandsubscribeto self-dispose cleanly:Beta Was this translation helpful? Give feedback.
All reactions