From c494c265f80fcb1800e83bc39fbfa4b58c754e96 Mon Sep 17 00:00:00 2001 From: Eugene Obrezkov Date: Wed, 8 Apr 2020 19:44:51 +0300 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20implement=20more=20user-?= =?UTF-8?q?friendly=20error=20handling=20in=20slides?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building a slide it will check all the semantics of slide declaration and all the references shapes <-> animations to prevent errors when showing off a presentation. This way, you will get all the errors of missed shapes or animations ahead-of-time. --- .../spec/AnimationBuilder.spec.ts | 5 +- .../kittik-slide/spec/ShapeBuilder.spec.ts | 5 +- packages/kittik-slide/spec/Slide.spec.ts | 82 ++++++++++++------- .../src/animation/AnimationBuilder.ts | 5 +- .../kittik-slide/src/shape/ShapeBuilder.ts | 5 +- packages/kittik-slide/src/slide/Slide.ts | 62 +++++++++----- 6 files changed, 112 insertions(+), 52 deletions(-) diff --git a/packages/kittik-slide/spec/AnimationBuilder.spec.ts b/packages/kittik-slide/spec/AnimationBuilder.spec.ts index 7aa7642..28dfe98 100644 --- a/packages/kittik-slide/spec/AnimationBuilder.spec.ts +++ b/packages/kittik-slide/spec/AnimationBuilder.spec.ts @@ -49,6 +49,9 @@ describe('animation builder', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore AnimationBuilder.start('Nonsense').end(); - }).toThrow('Animation "Nonsense" you tried to build does not exist'); + }).toThrow( + 'You tried to build an animation with the type "Nonsense". ' + + 'But the animation of this type is not implemented or you made a typo.' + ); }); }); diff --git a/packages/kittik-slide/spec/ShapeBuilder.spec.ts b/packages/kittik-slide/spec/ShapeBuilder.spec.ts index ccbb093..63e4688 100644 --- a/packages/kittik-slide/spec/ShapeBuilder.spec.ts +++ b/packages/kittik-slide/spec/ShapeBuilder.spec.ts @@ -61,6 +61,9 @@ describe('shape builder', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore ShapeBuilder.start('Nonsense').end(); - }).toThrow('Shape "Nonsense" you tried to build does not exist'); + }).toThrow( + 'You tried to build a shape with the type "Nonsense". ' + + 'But the shape of this type is not implemented or you made a typo.' + ); }); }); diff --git a/packages/kittik-slide/spec/Slide.spec.ts b/packages/kittik-slide/spec/Slide.spec.ts index 93ab25a..c57457f 100644 --- a/packages/kittik-slide/spec/Slide.spec.ts +++ b/packages/kittik-slide/spec/Slide.spec.ts @@ -84,42 +84,54 @@ describe('slide', () => { const cursor = new Canvas(); - expect( - () => new Slide( - cursor, { - name: 'Test', - shapes: [{ name: 'Test', type: 'unknown' }], - order: [{ shape: 'Test' }] - } - ) - ).toThrow('Shape "Test" (unknown) is unknown for me, maybe you made a typo?'); + expect(() => new Slide( + cursor, + { + name: 'Test', + shapes: [{ name: 'Test', type: 'unknown' }], + order: [{ shape: 'Test' }] + } + )).toThrow( + 'You have specified a shape with the name "Test" in slide "Test". ' + + 'This shape has an unknown type "unknown". ' + + 'Maybe you made a typo in "type" or tried to use a shape we do not have implemented.' + ); }); it('should properly throw an error if animation type is unknown', () => { expect.hasAssertions(); const cursor = new Canvas(); - expect( - () => new Slide( - cursor, { - name: 'Test', - shapes: [{ name: 'Test', type: 'Text' }], - animations: [{ name: 'Test', type: 'unknown' }], - order: [{ shape: 'Test' }] - } - ) - ).toThrow('Animation "Test" (unknown) is unknown for me, maybe you made a typo?'); + + expect(() => new Slide( + cursor, + { + name: 'Test', + shapes: [{ name: 'Test', type: 'Text' }], + animations: [{ name: 'Test', type: 'unknown' }], + order: [{ shape: 'Test' }] + } + )).toThrow( + 'You have specified an animation with the name "Test" in slide "Test". ' + + 'This animation has an unknown type "unknown". ' + + 'Maybe you made a typo in "type" or tried to use an animation we do not have implemented.' + ); }); it('should properly throw an error if trying to use shape name in ordering that does not exist', async () => { expect.hasAssertions(); const cursor = new Canvas(); - const slide = new Slide(cursor, { name: 'Test', shapes: [], order: [{ shape: 'Not Exists' }] }); + const slide = new Slide(cursor, { + name: 'Test', + shapes: [], + order: [{ shape: 'Not Exists' }] + }); await expect(slide.render()).rejects.toThrow( 'You specified shape "Not Exists" in slide "Test" as part of ordering, ' + - 'but it does not exist in shapes declaration.' + 'but it does not exist in shapes declaration for the slide. ' + + 'Maybe you forgot to create a shape you want to order or it is a typo in ordering itself.' ); }); @@ -214,7 +226,8 @@ describe('slide', () => { it('should properly instantiate an empty slide instance when nothing is passed but an empty arrays', () => { expect.hasAssertions(); - const slide = new Slide(null, { name: 'Test', order: [], shapes: [] }); + const cursor = new Canvas(); + const slide = new Slide(cursor, { name: 'Test', order: [], shapes: [] }); expect(slide.cursor).toBeInstanceOf(Canvas); expect(slide.shapes.size).toBe(0); expect(slide.animations.size).toBe(0); @@ -224,29 +237,42 @@ describe('slide', () => { it('should properly throw an error when trying to add shape that is already added', () => { expect.hasAssertions(); - const slide = new Slide(null, { name: 'Test', shapes: [{ name: 'Test', type: 'Text' }], order: [] }); - expect(() => slide.addShape('Test', Text.create())).toThrow('Shape "Test" already exists in slide'); + const canvas = new Canvas(); + const slide = new Slide(canvas, { name: 'Test', shapes: [{ name: 'Test', type: 'Text' }], order: [] }); + + expect(() => slide.addShape('Test', Text.create())).toThrow( + 'You are trying to add shape with the name "Test" into the slide "Test". ' + + 'But this shape already exists in slide "Test".' + ); }); it('should properly throw an error when trying to add animation that is already added', () => { expect.hasAssertions(); - const slide = new Slide(null, { + const canvas = new Canvas(); + const slide = new Slide(canvas, { name: 'Test', shapes: [], order: [], animations: [{ name: 'Test', type: 'Print' }] }); - expect(() => slide.addAnimation('Test', Print.create())).toThrow('Animation "Test" already exists in slide'); + expect(() => slide.addAnimation('Test', Print.create())).toThrow( + 'You are trying to add animation with the name "Test" into the slide "Test". ' + + 'But this animation already exists in slide "Test".' + ); }); it('should properly throw an error when trying to add ordering for the shape that is already added', () => { expect.hasAssertions(); - const slide = new Slide(null, { name: 'Test', shapes: [], order: [{ shape: 'Test' }] }); + const canvas = new Canvas(); + const slide = new Slide(canvas, { name: 'Test', shapes: [], order: [{ shape: 'Test' }] }); + expect(() => slide.addOrder('Test')).toThrow( - 'You already have an ordering for shape "Test" in slide "Test"' + 'You already have specified an ordering for shape "Test" in slide "Test". ' + + 'Adding another one with the same name does not make any sense. ' + + 'Did you make a typo in shape name or forgot that you already added a shape to ordering?' ); }); diff --git a/packages/kittik-slide/src/animation/AnimationBuilder.ts b/packages/kittik-slide/src/animation/AnimationBuilder.ts index 6b34d0d..b84edad 100644 --- a/packages/kittik-slide/src/animation/AnimationBuilder.ts +++ b/packages/kittik-slide/src/animation/AnimationBuilder.ts @@ -40,7 +40,10 @@ export class AnimationBuilder implements AnimationObject { public end (): Animationable { const ctr = ANIMATIONS.get(this.type); if (typeof ctr === 'undefined') { - throw new Error(`Animation "${this.type}" you tried to build does not exist`); + throw new Error( + `You tried to build an animation with the type "${this.type}". ` + + 'But the animation of this type is not implemented or you made a typo.' + ); } return ctr.fromObject(this); diff --git a/packages/kittik-slide/src/shape/ShapeBuilder.ts b/packages/kittik-slide/src/shape/ShapeBuilder.ts index bc2198d..edfd00a 100644 --- a/packages/kittik-slide/src/shape/ShapeBuilder.ts +++ b/packages/kittik-slide/src/shape/ShapeBuilder.ts @@ -70,7 +70,10 @@ export class ShapeBuilder implements ShapeObject { public end (): ShapeRenderable { const ctr = SHAPES.get(this.type); if (typeof ctr === 'undefined') { - throw new Error(`Shape "${this.type}" you tried to build does not exist`); + throw new Error( + `You tried to build a shape with the type "${this.type}". ` + + 'But the shape of this type is not implemented or you made a typo.' + ); } return ctr.fromObject(this); diff --git a/packages/kittik-slide/src/slide/Slide.ts b/packages/kittik-slide/src/slide/Slide.ts index f53dd80..fe81772 100644 --- a/packages/kittik-slide/src/slide/Slide.ts +++ b/packages/kittik-slide/src/slide/Slide.ts @@ -47,21 +47,24 @@ export class Slide { } } - public static create (cursor: Canvas, declaration: SlideDeclaration): Slide { + public static create (cursor?: Canvas, declaration?: SlideDeclaration): Slide { return new this(cursor, declaration); } - public static fromObject (obj: SlideDeclaration, cursor: Canvas): Slide { + public static fromObject (obj: SlideDeclaration, cursor?: Canvas): Slide { return this.create(cursor, obj); } - public static fromJSON (json: string, cursor: Canvas): Slide { + public static fromJSON (json: string, cursor?: Canvas): Slide { return this.fromObject(JSON.parse(json), cursor); } public addShape (name: string, shape: ShapeRenderable, toOverride = false): void { if (this.shapes.has(name) && !toOverride) { - throw new Error(`Shape "${name}" already exists in slide "${this.name}"`); + throw new Error( + `You are trying to add shape with the name "${name}" into the slide "${this.name}". ` + + `But this shape already exists in slide "${this.name}".` + ); } this.shapes.set(name, shape); @@ -69,37 +72,53 @@ export class Slide { public addAnimation (name: string, animation: Animationable, toOverride = false): void { if (this.animations.has(name) && !toOverride) { - throw new Error(`Animation "${name}" already exists in slide "${this.name}"`); + throw new Error( + `You are trying to add animation with the name "${name}" into the slide "${this.name}". ` + + `But this animation already exists in slide "${this.name}".` + ); } this.animations.set(name, animation); } public addOrder (shape: string, animations: string[] = []): void { - if (this.order.some((order: OrderDeclaration) => order.shape === shape)) { - throw new Error(`You already have an ordering for shape "${shape}" in slide "${this.name}"`); + if (this.order.some((order) => order.shape === shape)) { + throw new Error( + `You already have specified an ordering for shape "${shape}" in slide "${this.name}". ` + + 'Adding another one with the same name does not make any sense. ' + + 'Did you make a typo in shape name or forgot that you already added a shape to ordering?' + ); + } + + const unknownAnimations = animations.filter((animation) => !this.animations.has(animation)); + if (unknownAnimations.length > 0) { + throw new Error( + `You have provided animations for the shape "${shape}" in slide "${this.name}". ` + + `But, some of them could not be found in the slide "${this.name}". ` + + `These animations are: [${unknownAnimations.join(', ')}]. ` + + `Please, check if the animations from the list are declared in slide "${this.name}".` + ); } this.order.push({ animations, shape }); } - // eslint-disable-next-line max-statements public async render (): Promise { - const { shapes } = this; - const { animations } = this; + const { animations, shapes } = this; const shapesToRender: ShapeRenderable[] = []; const sequence: Array<() => void> = []; + const onTick = (): void => this.renderShapes(shapesToRender); // We need to re-render shapes each time when some of the animations made a tick // Hence, made an update in shape properties that we need to reflect on canvas - animations.forEach((animation: Animationable) => animation.on('tick', () => this.renderShapes(shapesToRender))); + animations.forEach((animation) => animation.on('tick', onTick)); for (const order of this.order) { const shapeToRender = shapes.get(order.shape); if (typeof shapeToRender === 'undefined') { throw new Error( `You specified shape "${order.shape}" in slide "${this.name}" ` + - 'as part of ordering, but it does not exist in shapes declaration.\n' + + 'as part of ordering, but it does not exist in shapes declaration for the slide. ' + 'Maybe you forgot to create a shape you want to order or it is a typo in ordering itself.' ); } @@ -117,9 +136,9 @@ export class Slide { } // When all of the rendering and animation is done - we can freely remove the listeners from animations - sequence.push(() => animations.forEach((animation: Animationable) => animation.removeAllListeners())); + sequence.push(() => animations.forEach((animation) => animation.removeListener('tick', onTick))); - // We can't allow Promise.all() here or anything that could render the shapes concurrently or parallel + // We can't allow Promise.all() here or anything that could render the shapes concurrently // Hence, we need to reduce the sequence to the chain of promises, so we can get waterfall rendering return await sequence.reduce(async (promise, item) => await promise.then(item), Promise.resolve()); } @@ -141,12 +160,14 @@ export class Slide { } private initShapes (declaration: ShapeDeclaration[]): void { - declaration.forEach((shapeDeclaration: ShapeDeclaration) => { + declaration.forEach((shapeDeclaration) => { const ctor = SHAPES.get(shapeDeclaration.type as ShapeType); if (typeof ctor === 'undefined') { throw new Error( - `Shape "${shapeDeclaration.name}" (${shapeDeclaration.type}) is unknown for me, maybe you made a typo?` + `You have specified a shape with the name "${shapeDeclaration.name}" in slide "${this.name}". ` + + `This shape has an unknown type "${shapeDeclaration.type}". ` + + 'Maybe you made a typo in "type" or tried to use a shape we do not have implemented.' ); } @@ -155,13 +176,14 @@ export class Slide { } private initAnimations (declaration: AnimationDeclaration[]): void { - declaration.forEach((animationDeclaration: AnimationDeclaration) => { + declaration.forEach((animationDeclaration) => { const ctor = ANIMATIONS.get(animationDeclaration.type as AnimationType); if (typeof ctor === 'undefined') { throw new Error( - `Animation "${animationDeclaration.name}" (${animationDeclaration.type}) is unknown for me, ` + - 'maybe you made a typo?' + `You have specified an animation with the name "${animationDeclaration.name}" in slide "${this.name}". ` + + `This animation has an unknown type "${animationDeclaration.type}". ` + + 'Maybe you made a typo in "type" or tried to use an animation we do not have implemented.' ); } @@ -171,7 +193,7 @@ export class Slide { private renderShapes (shapes: ShapeRenderable[]): void { this.cursor.eraseScreen(); - shapes.forEach((shape: ShapeRenderable) => shape.render(this.cursor)); + shapes.forEach((shape) => shape.render(this.cursor)); this.cursor.flush(); } }