refactor(interfaces): Make type predicates more robust by cpcallen · Pull Request #9150 · RaspberryPiFoundation/blockly

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen · 2025-06-17T13:49:18Z

The basics

I validated my changes

The details

Proposed Changes

Make type predicates for interface types more robust:

Always use typeof … === 'function' when testing for presence of methods.
Test for presence of all non-optional members.
Accept input of type any but always check that the value is neither null nor undefined.

Reason for Changes

Because many of these interfaces define developer-supplied types, making the corresponding type predicates more exacting will help ensure that we do not inadvertently accept a value that only partially conforms to the interface.

For example, given some object

constobj={a: 0,b: 'hello',c(){console.log('hello');}d: undefined,// There is no e.}

the typical tests we use will all return true for almost any non-missing value:

obj.a!==undefined// trueobj.b!==undefined// trueobj.c!==undefined// trueobj.d!==undefined// falseobj.e!==undefined// false

while a lazier test might catch a primitive being supplied where a method was expected, at least if the primitive is falsey:

Boolean(obj.a)// FALSEBoolean(obj.b)// trueBoolean(obj.c)// trueBoolean(obj.d)// falseBoolean(obj.e)// false

whereas using typeof will ensure that, at least for primitive- or method-valued members of the interface, the type is approximately as expected:

typeofobj.a==='number'// truetypeofobj.a==='string'// falsetypeofobj.a==='function'// falsetypeofobj.b==='function'// falsetypeofobj.c==='function'// truetypeofobj.d==='function'// falsetypeofobj.e==='function'// false

Notably, the only values whose typeof is 'function' that are not callable as a method are ES6 class constructors (e.g., class C {}), which need to be invoked with new.

Test Coverage

Passes npm test.

Additional Information

One possible argument against this change might be that the type predicates will be slightly more expensive to invoke—though I suspect that in most situations the JIT will optimise away most of the actual checks. Likely only use of these predicates inside hotspot inner loops is likely to be of any measurable effect, but it would be useful if we had some benchmarks so we could actually test to see what effect if any this change might have.

Testing for 'name' in object or obj.name !== undefined only checks for the existence of the property (and in the latter case that the property is not set to undefined). That's fine if the interface specifies a property of indeterminate type, but in the usual case that the interface member is a method we can do one better and check to make sure the property's value is callable.

Since most type predicates take an argument of type any but then check for the existence of certain properties, explicitly check that the argument is not null or undefined (or check implicitly by calling another type predicate that does so first, which necessitates adding a few casts because tsc infers the type of the argument too narrowly).

This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen · 2025-06-17T14:52:00Z

Marking this as draft for now because it turns out that some of our own test mocks do not conform to their specified types!

Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

Add (test) runtime assertions that: - isFocusableNode(MockFocusable) returns true - isIcon(MockIcon) returns true - hasBubble(MockBubbleIcon) returns true - isCommentIcon(MockCommentIcon) returns true (The latter is currently failing because Blockly is undefined when isCommentIcon calls the MockCommentIcon's getType method.)

For some reason the global Blockly binding is not visible at the time when isCommentIcon calls MockCommentIcon's getType method, and presumably this problem would apply to getBubbleSize too, so directly import the required items.

This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

cpcallen added 3 commits June 17, 2025 14:26

fix(interfaces): Add missing check to hasBubble type predicate
cdde749
This appears to have inadvertently been omitted in PR RaspberryPiFoundation#9004.

cpcallen requested a review from a team as a code owner June 17, 2025 13:49

cpcallen requested a review from rachel-fenichel June 17, 2025 13:49

github-actionsBot assigned rachel-fenichel Jun 17, 2025

github-actionsBot added the PR: refactor Refactors code label Jun 17, 2025

cpcallen added 2 commits June 17, 2025 15:14

fix(interfaces): Fix misplaced typeof
f83af14

fix: Fix typos in JSDocs
35276c1

cpcallen marked this pull request as draft June 17, 2025 14:51

cpcallen added 5 commits June 19, 2025 16:48

fix(tests): Make Mocks conform to corresponding interfaces
32dc18a
Introduce a new MockFocusable, and add methods to MockIcon, MockBubbleIcon and MockComment, so that they fulfil the IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces respectively.

refactor(tests): Make MockCommentIcon a MockBubbleIcon
feec7cc
This slightly simplifies it and makes it less likely to accidentally stop conforming to IHasBubble.

fix(interfaces): Fix incorrect check in isSelectable
a909d95
Fix an error which caused ISelectable instances to fail isSelectable() checks, one of the results of which is that Blockly.common.getSelected() would generally return null. Whoops!

cpcallen marked this pull request as ready for review June 19, 2025 16:29

cpcallen requested a review from maribethb June 19, 2025 16:29

github-actionsBot assigned maribethb Jun 19, 2025

maribethb approved these changes Jun 23, 2025
View reviewed changes

cpcallen merged commit f4dbea0 into RaspberryPiFoundation:developJun 25, 2025

cpcallen deleted the fix/interface-predicates branch June 25, 2025 11:49

Uh oh!

Conversation

cpcallen commented Jun 17, 2025

The basics

The details

Proposed Changes

Reason for Changes

Test Coverage

Additional Information

Uh oh!

cpcallen commented Jun 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

cpcallen commented Jun 17, 2025

The basics

The details

Proposed Changes

Reason for Changes

Test Coverage

Additional Information

Uh oh!

cpcallen commented Jun 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

cpcallen commented Jun 17, 2025

The basics

The details

Proposed Changes

Reason for Changes

Test Coverage

Additional Information

Uh oh!

cpcallen commented Jun 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

cpcallen commented Jun 17, 2025

The basics

The details

Proposed Changes

Reason for Changes

Test Coverage

Additional Information

Uh oh!

cpcallen commented Jun 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants