Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Activate strictBindCallApply and strictFunctionTypes compiler rules #2880

Merged
merged 2 commits into from
Apr 12, 2019
Merged

Activate strictBindCallApply and strictFunctionTypes compiler rules #2880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2e15ae
Fix firebase strictFunctionTypes violation
lindner Apr 10, 2019
ea28339
Activate strictBindCallApply and strictFunctionTypes compiler rules
lindner Apr 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions config/tsconfig.base.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
"noImplicitThis": true,
"module": "es2015",
"moduleResolution": "Node",
"strictBindCallApply": true,
"strictFunctionTypes": true,

"allowJs": false,
"sourceMap": true,
Expand Down
26 changes: 18 additions & 8 deletions src/runtime/api-channel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ import {Arc} from './arc.js';
import {DevtoolsConnection} from './debug/devtools-connection.js';
import {OuterPortAttachment} from './debug/outer-port-attachment.js';
import {Handle} from './handle.js';
import {ParticleSpec} from './particle-spec.js';
import {ParticleSpec, SerializedParticleSpec} from './particle-spec.js';
import {Particle} from './particle.js';
import * as recipeHandle from './recipe/handle.js';
import * as recipeParticle from './recipe/particle.js';
import {StorageProxy} from './storage-proxy.js';
import {SerializedModelEntry} from './storage/crdt-collection-model.js';
import {StorageProviderBase} from './storage/storage-provider-base.js';
import {Type} from './type.js';
import {PropagatedException} from './arc-exceptions.js';
import {Type, TypeLiteral} from './type.js';
import {PropagatedException, SerializedPropagatedException} from './arc-exceptions.js';

enum MappingType {Mapped, LocalMapped, RemoteMapped, Direct, ObjectMap, List, ByLiteral}

Expand All @@ -32,15 +32,25 @@ interface MappingInfo {
redundant?: boolean;
value?: MappingInfo;
key?: MappingInfo;
converter?: Literalizer;
converter?: Literalizer | LiteralizerParticleSpec | LiteralizerPropagatedException;
identifier?: boolean;
ignore?: boolean;
}

// TODO(shans): are there better types that I can use for this?
interface Literalizer {
prototype: {toLiteral: () => {}};
fromLiteral: ({}) => {};
prototype: {toLiteral: () => TypeLiteral};
fromLiteral: (typeliteral: TypeLiteral) => Type;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a little hacky, but describes the current usage. Should ParticleSpec/PropagatedException implement Type/TypeLiteral instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

better to templatize literalizer on the literal type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to parameterize Literalizer, but none of them survived the use of decorators with any fidelity.

I also started down the path of creating a new Literalizable interface but ran into the same issues with adding Types to EnityType (namely lack of static methods in interfaces microsoft/TypeScript#14600) Especially with generics.

Since the latter solution is preferred, but also much more invasive I'll spin those changes out to another cleanup issue.

interface LiteralizerParticleSpec {
prototype: {toLiteral: () => SerializedParticleSpec};
fromLiteral: (spec: SerializedParticleSpec) => ParticleSpec;
}

interface LiteralizerPropagatedException {
prototype: {toLiteral: () => SerializedPropagatedException};
fromLiteral: (exception: SerializedPropagatedException) => PropagatedException;
}

const targets = new Map<{}, Map<string, MappingInfo[]>>();
Expand All @@ -67,7 +77,7 @@ function Mapped(target: {}, propertyKey: string, parameterIndex: number) {
set(target.constructor, propertyKey, parameterIndex, {type: MappingType.Mapped});
}

function ByLiteral(constructor: Literalizer) {
function ByLiteral(constructor: Literalizer | LiteralizerParticleSpec | LiteralizerPropagatedException) {
return (target: {}, propertyKey: string, parameterIndex: number) => {
const info: MappingInfo = {type: MappingType.ByLiteral, converter: constructor};
set(target.constructor, propertyKey, parameterIndex, info);
Expand Down Expand Up @@ -415,7 +425,7 @@ export abstract class PECOuterPort extends APIPort {
UIEvent(@Mapped particle: recipeParticle.Particle, @Direct slotName: string, @Direct event: {}) {}
SimpleCallback(@RemoteMapped callback: number, @Direct data: {}) {}
AwaitIdle(@Direct version: number) {}
StartRender(@Mapped particle: recipeParticle.Particle, @Direct slotName: string, @ObjectMap(MappingType.Direct, MappingType.Direct) providedSlots: {[index: string]: string}, @List(MappingType.Direct) contentTypes: string[]) {}
StartRender(@Mapped particle: recipeParticle.Particle, @Direct slotName: string, @ObjectMap(MappingType.Direct, MappingType.Direct) providedSlots: Map<string, string>, @List(MappingType.Direct) contentTypes: string[]) {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that the new checks caught this type mismatch. (Should be Map<string,string>)

Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

StopRender(@Mapped particle: recipeParticle.Particle, @Direct slotName: string) {}

abstract onRender(particle: recipeParticle.Particle, slotName: string, content: string);
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/arc-exceptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {Particle} from './particle';
* http://polymer.github.io/PATENTS.txt
*/

type SerializedPropagatedException = {
export type SerializedPropagatedException = {
exceptionType: string,
cause: {name: string, message: string, stack: string}, // Serialized Error.
method: string,
Expand Down
16 changes: 14 additions & 2 deletions src/runtime/particle-execution-host.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ import {SlotComposer} from './slot-composer.js';
import {StorageProviderBase} from './storage/storage-provider-base.js';
import {Type} from './type.js';

export type StartRenderOptions = {
particle: Particle;
slotName: string;
providedSlots: Map<string, string>;
contentTypes: string[];
};

export type StopRenderOptions = {
particle: Particle;
slotName: string;
};

export class ParticleExecutionHost {
private _apiPort : PECOuterPort;
close : () => void;
Expand Down Expand Up @@ -281,11 +293,11 @@ export class ParticleExecutionHost {
this._apiPort.InstantiateParticle(particle, particle.id.toString(), particle.spec, stores);
}

startRender({particle, slotName, providedSlots, contentTypes}: {particle: Particle, slotName: string, providedSlots: {[index: string]: string}, contentTypes: string[]}) {
startRender({particle, slotName, providedSlots, contentTypes}: StartRenderOptions): void {
this._apiPort.StartRender(particle, slotName, providedSlots, contentTypes);
}

stopRender({particle, slotName}: {particle: Particle, slotName: string}) {
stopRender({particle, slotName}: StopRenderOptions): void {
this._apiPort.StopRender(particle, slotName);
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/particle-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class ProvideSlotConnectionSpec {
}
}

type SerializedParticleSpec = {
export type SerializedParticleSpec = {
name: string,
id?: string,
verbs: string[],
Expand Down
10 changes: 7 additions & 3 deletions src/runtime/slot-consumer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ import {assert} from '../platform/assert-web.js';

import {Arc} from './arc.js';
import {Description} from './description.js';
import {Particle} from './recipe/particle.js';
import {SlotConnection} from './recipe/slot-connection.js';
import {HostedSlotContext, ProvidedSlotContext, SlotContext} from './slot-context.js';
import {StartRenderOptions, StopRenderOptions} from './particle-execution-host.js';

export interface Content {
templateName?: string | Map<string, string>;
Expand Down Expand Up @@ -41,8 +43,8 @@ export class SlotConsumer {
slotContext: SlotContext;
readonly directlyProvidedSlotContexts: ProvidedSlotContext[] = [];
readonly hostedSlotContexts: HostedSlotContext[] = [];
startRenderCallback: ({}) => void;
stopRenderCallback: ({}) => void;
startRenderCallback: (options: StartRenderOptions) => void;
stopRenderCallback: (options: StopRenderOptions) => void;
eventHandler: ({}) => void;
readonly containerKind?: string;
// Contains `container` and other modality specific rendering information
Expand Down Expand Up @@ -149,10 +151,12 @@ export class SlotConsumer {

startRender() {
if (this.consumeConn && this.startRenderCallback) {
const providedSlots = new Map(this.allProvidedSlotContexts.map(context => ([context.name, context.id] as [string, string])));

this.startRenderCallback({
particle: this.consumeConn.particle,
slotName: this.consumeConn.name,
providedSlots: new Map(this.allProvidedSlotContexts.map(context => ([context.name, context.id] as [string, string]))),
providedSlots,
contentTypes: this.constructRenderRequest()
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/storage/firebase/firebase-storage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ class FirebaseVariable extends FirebaseStorageProvider implements VariableStorag
private pendingWrites: {storageKey: string, value: {}}[] = [];
wasConnect: boolean; // for debugging
private resolveInitialized: () => void;
private readonly valueChangeCallback: ({}) => void;
private readonly valueChangeCallback: (dataSnapshot: firebase.database.DataSnapshot, s?: string) => void;

constructor(type, storageEngine, id, reference, firebaseKey, shouldExist) {
super(type, storageEngine, id, reference, firebaseKey);
Expand Down Expand Up @@ -651,7 +651,7 @@ class FirebaseCollection extends FirebaseStorageProvider implements CollectionSt
private pendingWrites: {value: {}, storageKey: string}[] = [];
private resolveInitialized: () => void;
private localKeyId = Date.now();
private readonly valueChangeCallback: ({}) => void;
private readonly valueChangeCallback: (dataSnapshot: firebase.database.DataSnapshot, s?: string) => void;

constructor(type, storageEngine, id, reference, firebaseKey) {
super(type, storageEngine, id, reference, firebaseKey);
Expand Down