Skip to content

Commit

Permalink
Revert "Add blocking signal feature (#443)" (#522)
Browse files Browse the repository at this point in the history
This reverts commit c90adee.
  • Loading branch information
fcollonval authored Jan 23, 2023
1 parent a00b585 commit d7ac559
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 200 deletions.
3 changes: 1 addition & 2 deletions packages/signaling/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
},
"dependencies": {
"@lumino/algorithm": "^2.0.0-beta.0",
"@lumino/coreutils": "^2.0.0-beta.0",
"@lumino/properties": "^2.0.0-beta.0"
"@lumino/coreutils": "^2.0.0-beta.0"
},
"devDependencies": {
"@microsoft/api-extractor": "^7.6.0",
Expand Down
79 changes: 5 additions & 74 deletions packages/signaling/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
|----------------------------------------------------------------------------*/
import { ArrayExt, find } from '@lumino/algorithm';
import { PromiseDelegate } from '@lumino/coreutils';
import { AttachedProperty } from '@lumino/properties';

/**
* A type alias for a slot function.
Expand All @@ -33,16 +32,6 @@ export type Slot<T, U> = (sender: T, args: U) => void;
* subscribers are invoked whenever the publisher emits the signal.
*/
export interface ISignal<T, U> {
/**
* Block the signal during the execution of a callback.
*
* ### Notes
* The callback function must be synchronous.
*
* @param fn The callback during which the signal is blocked
*/
block(fn: () => void): void;

/**
* Connect a slot to the signal.
*
Expand Down Expand Up @@ -154,23 +143,6 @@ export class Signal<T, U> implements ISignal<T, U> {
*/
readonly sender: T;

/**
* Block the signal during the execution of a callback.
*
* ### Notes
* The callback function must be synchronous.
*
* @param fn The callback during which the signal is blocked
*/
block(fn: () => void): void {
this.blocked++;
try {
fn();
} finally {
this.blocked--;
}
}

/**
* Connect a slot to the signal.
*
Expand Down Expand Up @@ -210,41 +182,14 @@ export class Signal<T, U> implements ISignal<T, U> {
* Exceptions thrown by connected slots will be caught and logged.
*/
emit(args: U): void {
if (!this.blocked) {
Private.emit(this, args);
}
Private.emit(this, args);
}

/**
* If `blocked` is not `0`, the signal will not emit.
*/
protected blocked = 0;
}

/**
* The namespace for the `Signal` class statics.
*/
export namespace Signal {
/**
* Block all signals emitted by an object during
* the execution of a callback.
*
* ### Notes
* The callback function must be synchronous.
*
* @param sender The signals sender
* @param fn The callback during which all signals are blocked
*/
export function blockAll(sender: unknown, fn: () => void): void {
const { blockedProperty } = Private;
blockedProperty.set(sender, blockedProperty.get(sender) + 1);
try {
fn();
} finally {
blockedProperty.set(sender, blockedProperty.get(sender) - 1);
}
}

/**
* Remove all connections between a sender and receiver.
*
Expand Down Expand Up @@ -414,12 +359,10 @@ export class Stream<T, U> extends Signal<T, U> implements IStream<T, U> {
* @param args - The args to pass to the connected slots.
*/
emit(args: U): void {
if (!this.blocked) {
const pending = this._pending;
const next = (this._pending = new PromiseDelegate());
pending.resolve({ args, next });
super.emit(args);
}
const pending = this._pending;
const next = (this._pending = new PromiseDelegate());
pending.resolve({ args, next });
super.emit(args);
}

/**
Expand Down Expand Up @@ -677,10 +620,6 @@ namespace Private {
* Exceptions thrown by connected slots will be caught and logged.
*/
export function emit<T, U>(signal: Signal<T, U>, args: U): void {
if (Private.blockedProperty.get(signal.sender) > 0) {
return;
}

// If there are no receivers, there is nothing to do.
let receivers = receiversForSender.get(signal.sender);
if (!receivers || receivers.length === 0) {
Expand Down Expand Up @@ -821,12 +760,4 @@ namespace Private {
function isDeadConnection(connection: IConnection): boolean {
return connection.signal === null;
}

/**
* A property indicating a sender has been blocked if its value is not 0.
*/
export const blockedProperty = new AttachedProperty<unknown, number>({
name: 'blocked',
create: () => 0
});
}
121 changes: 1 addition & 120 deletions packages/signaling/tests/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,58 +65,6 @@ describe('@lumino/signaling', () => {
});
});

describe('#block()', () => {
it('should block the signal emission', () => {
let obj = new TestObject();
let handler1 = new TestHandler();
let handler2 = new TestHandler();
obj.two.connect(handler1.onTwo, handler1);
obj.two.connect(handler2.onTwo, handler2);

obj.two.block(() => {
obj.two.emit(4);
});

expect(handler1.twoSender).to.equal(null);
expect(handler2.twoSender).to.equal(null);
expect(handler1.twoValue).to.equal(0);
expect(handler2.twoValue).to.equal(0);

obj.two.emit(15);
expect(handler1.twoSender).to.equal(obj);
expect(handler2.twoSender).to.equal(obj);
expect(handler1.twoValue).to.equal(15);
expect(handler2.twoValue).to.equal(15);
});

it('should block the signal emission for nested loop', () => {
let obj = new TestObject();
let handler1 = new TestHandler();
let handler2 = new TestHandler();
obj.two.connect(handler1.onTwo, handler1);
obj.two.connect(handler2.onTwo, handler2);

obj.two.block(() => {
obj.two.emit(4);
obj.two.block(() => {
obj.two.emit(42);
});
obj.two.emit(6);
});

expect(handler1.twoSender).to.equal(null);
expect(handler2.twoSender).to.equal(null);
expect(handler1.twoValue).to.equal(0);
expect(handler2.twoValue).to.equal(0);

obj.two.emit(15);
expect(handler1.twoSender).to.equal(obj);
expect(handler2.twoSender).to.equal(obj);
expect(handler1.twoValue).to.equal(15);
expect(handler2.twoValue).to.equal(15);
});
});

describe('#connect()', () => {
it('should return true on success', () => {
let obj = new TestObject();
Expand Down Expand Up @@ -352,69 +300,6 @@ describe('@lumino/signaling', () => {
});
});

describe('.blockAll()', () => {
it('should block all signals from a given sender', () => {
let obj = new TestObject();
let handler1 = new TestHandler();
let handler2 = new TestHandler();
obj.one.connect(handler1.onOne, handler1);
obj.one.connect(handler2.onOne, handler2);
obj.two.connect(handler1.onTwo, handler1);
obj.two.connect(handler2.onTwo, handler2);

Signal.blockAll(obj, () => {
obj.one.emit(undefined);
obj.two.emit(42);
});
expect(handler1.oneCount).to.equal(0);
expect(handler2.oneCount).to.equal(0);
expect(handler1.twoValue).to.equal(0);
expect(handler2.twoValue).to.equal(0);

obj.one.emit(undefined);
obj.two.emit(42);
expect(handler1.oneCount).to.equal(1);
expect(handler2.oneCount).to.equal(1);
expect(handler1.twoValue).to.equal(42);
expect(handler2.twoValue).to.equal(42);
});

it('should block all signals from a given sender for nested loop', () => {
let obj = new TestObject();
let handler1 = new TestHandler();
let handler2 = new TestHandler();
obj.one.connect(handler1.onOne, handler1);
obj.one.connect(handler2.onOne, handler2);
obj.two.connect(handler1.onTwo, handler1);
obj.two.connect(handler2.onTwo, handler2);

Signal.blockAll(obj, () => {
obj.one.emit(undefined);
obj.two.emit(4);

Signal.blockAll(obj, () => {
obj.one.emit(undefined);
obj.two.emit(12);
});

obj.one.emit(undefined);
obj.two.emit(6);
});

expect(handler1.oneCount).to.equal(0);
expect(handler2.oneCount).to.equal(0);
expect(handler1.twoValue).to.equal(0);
expect(handler2.twoValue).to.equal(0);

obj.one.emit(undefined);
obj.two.emit(42);
expect(handler1.oneCount).to.equal(1);
expect(handler2.oneCount).to.equal(1);
expect(handler1.twoValue).to.equal(42);
expect(handler2.twoValue).to.equal(42);
});
});

describe('.disconnectBetween()', () => {
it('should clear all connections between a sender and receiver', () => {
let obj = new TestObject();
Expand Down Expand Up @@ -585,7 +470,7 @@ describe('@lumino/signaling', () => {

describe('Stream', () => {
describe('#[Symbol.asyncIterator]()', () => {
it('should yield emissions and respect blocking', async () => {
it('should yield emissions', async () => {
const stream = new Stream<unknown, string>({});
const input = 'async';
const expected = 'aINTERRUPTEDsync';
Expand All @@ -608,9 +493,7 @@ describe('@lumino/signaling', () => {
stream.emit('D');
}
});
wait.then(() => stream.block(() => stream.emit('BLOCKED EMISSION 1')));
input.split('').forEach(x => wait.then(() => stream.emit(x)));
wait.then(() => stream.block(() => stream.emit('BLOCKED EMISSION 2')));
wait.then(() => stream.stop());
for await (const letter of stream) {
emitted = emitted.concat(letter);
Expand All @@ -634,9 +517,7 @@ describe('@lumino/signaling', () => {
stream.emit('M');
}
});
wait.then(() => stream.block(() => stream.emit('BLOCKED EMISSION 1')));
input.split('').forEach(x => wait.then(() => stream.emit(x)));
wait.then(() => stream.block(() => stream.emit('BLOCKED EMISSION 2')));
wait.then(() => stream.stop());

const it = stream[Symbol.asyncIterator]();
Expand Down
4 changes: 0 additions & 4 deletions review/api/signaling.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

// @public
export interface ISignal<T, U> {
block(fn: () => void): void;
connect(slot: Slot<T, U>, thisArg?: any): boolean;
disconnect(slot: Slot<T, U>, thisArg?: any): boolean;
}
Expand All @@ -18,8 +17,6 @@ export interface IStream<T, U> extends ISignal<T, U>, AsyncIterable<U> {
// @public
export class Signal<T, U> implements ISignal<T, U> {
constructor(sender: T);
block(fn: () => void): void;
protected blocked: number;
connect(slot: Slot<T, U>, thisArg?: unknown): boolean;
disconnect(slot: Slot<T, U>, thisArg?: unknown): boolean;
emit(args: U): void;
Expand All @@ -28,7 +25,6 @@ export class Signal<T, U> implements ISignal<T, U> {

// @public
export namespace Signal {
export function blockAll(sender: unknown, fn: () => void): void;
export function clearData(object: unknown): void;
export function disconnectAll(object: unknown): void;
export function disconnectBetween(sender: unknown, receiver: unknown): void;
Expand Down

0 comments on commit d7ac559

Please sign in to comment.