Skip to content

Commit

Permalink
Chore: Eliminate redundant handling code for extensions (pixijs#8485)
Browse files Browse the repository at this point in the history
  • Loading branch information
bigtimebuddy committed Jul 14, 2022
1 parent 6ff95ce commit 84096a7
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 70 deletions.
25 changes: 1 addition & 24 deletions packages/app/src/Application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,27 +185,4 @@ export class Application
}
}

extensions.handle(
ExtensionType.Application,
(extension) =>
{
const plugins = Application._plugins;
const plugin = extension.ref as unknown as IApplicationPlugin;

if (!plugins.includes(plugin))
{
plugins.push(plugin);
}
},
(extension) =>
{
const plugins = Application._plugins;
const plugin = extension.ref as unknown as IApplicationPlugin;
const index = plugins.indexOf(plugin);

if (index !== -1)
{
plugins.splice(index, 1);
}
}
);
extensions.handleByList(ExtensionType.Application, Application._plugins);
19 changes: 4 additions & 15 deletions packages/assets/src/Assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -774,21 +774,10 @@ export class AssetsClass
export const Assets = new AssetsClass();

// Handle registration of extensions
extensions.handle(
ExtensionType.LoadParser,
(extension) => { Assets.loader.addParser(extension.ref); },
(extension) => { Assets.loader.removeParser(extension.ref); }
);
extensions.handle(
ExtensionType.ResolveParser,
(extension) => { Assets.resolver.addUrlParser(extension.ref); },
(extension) => { Assets.resolver.removeUrlParser(extension.ref); }
);
extensions.handle(
ExtensionType.CacheParser,
(extension) => { Assets.cache.addParser(extension.ref); },
(extension) => { Assets.cache.removeParser(extension.ref); }
);
extensions
.handleByList(ExtensionType.LoadParser, Assets.loader.parsers)
.handleByList(ExtensionType.ResolveParser, Assets.resolver.parsers)
.handleByList(ExtensionType.CacheParser, Assets.cache.parsers);

extensions.add(
loadTextures,
Expand Down
5 changes: 5 additions & 0 deletions packages/assets/src/resolver/Resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ export class Resolver
return this._basePath;
}

public get parsers(): ResolveURLParser[]
{
return this._parsers;
}

/** Used for testing, this resets the resolver to its initial state */
public reset(): void
{
Expand Down
6 changes: 1 addition & 5 deletions packages/canvas-renderer/src/CanvasRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,4 @@ export class CanvasRenderer extends AbstractRenderer
}
}

extensions.handle(
ExtensionType.CanvasRendererPlugin,
(extension) => { CanvasRenderer.__plugins[extension.name] = extension.ref; },
(extension) => { delete CanvasRenderer.__plugins[extension.name]; }
);
extensions.handleByMap(ExtensionType.CanvasRendererPlugin, CanvasRenderer.__plugins);
6 changes: 1 addition & 5 deletions packages/core/src/Renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -642,8 +642,4 @@ export class Renderer extends AbstractRenderer
}

// Handle registration of extensions
extensions.handle(
ExtensionType.RendererPlugin,
(extension) => { Renderer.__plugins[extension.name] = extension.ref; },
(extension) => { delete Renderer.__plugins[extension.name]; }
);
extensions.handleByMap(ExtensionType.RendererPlugin, Renderer.__plugins);
60 changes: 60 additions & 0 deletions packages/core/src/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* @property {string} RendererPlugin - Plugins for Renderer
* @property {string} CanvasRendererPlugin - Plugins for CanvasRenderer
* @property {string} Loader - Plugins to use with Loader
* @property {string} LoadParser - Parsers for Assets loader.
* @property {string} ResolveParser - Parsers for Assets resolvers.
* @property {string} CacheParser - Parsers for Assets cache.
*/
enum ExtensionType
// eslint-disable-next-line @typescript-eslint/indent
Expand Down Expand Up @@ -111,18 +114,22 @@ const extensions = {
/**
* Remove extensions from PixiJS.
* @param extensions - Extensions to be removed.
* @returns {PIXI.extensions} For chaining.
*/
remove(...extensions: Array<ExtensionFormatLoose | any>)
{
extensions.map(normalizeExtension).forEach((ext) =>
{
ext.type.forEach((type) => this._removeHandlers[type]?.(ext));
});

return this;
},

/**
* Register new extensions with PixiJS.
* @param extensions - The spread of extensions to add to PixiJS.
* @returns {PIXI.extensions} For chaining.
*/
add(...extensions: Array<ExtensionFormatLoose | any>)
{
Expand All @@ -145,13 +152,16 @@ const extensions = {
}
});
});

return this;
},

/**
* Internal method to handle extensions by name.
* @param type - The extension type.
* @param onAdd - Function for handling when extensions are added/registered passes {@link PIXI.ExtensionFormat}.
* @param onRemove - Function for handling when extensions are removed/unregistered passes {@link PIXI.ExtensionFormat}.
* @returns {PIXI.extensions} For chaining.
*/
handle(type: ExtensionType, onAdd: ExtensionHandler, onRemove: ExtensionHandler)
{
Expand All @@ -177,6 +187,56 @@ const extensions = {
queue[type].forEach((ext) => onAdd(ext));
delete queue[type];
}

return this;
},

/**
* Handle a type, but using a map by `name` property.
* @param type - Type of extension to handle.
* @param map - The object map of named extensions.
* @returns {PIXI.extensions} For chaining.
*/
handleByMap(type: ExtensionType, map: Record<string, any>)
{
return this.handle(type,
(extension) =>
{
map[extension.name] = extension.ref;
},
(extension) =>
{
delete map[extension.name];
}
);
},

/**
* Handle a type, but using a list of extensions.
* @param type - Type of extension to handle.
* @param list - The list of extensions.
* @returns {PIXI.extensions} For chaining.
*/
handleByList(type: ExtensionType, list: any[])
{
return this.handle(
type,
(extension) =>
{
list.push(extension.ref);
// TODO: remove me later, only added for @pixi/loaders
extension.ref.add?.();
},
(extension) =>
{
const index = list.indexOf(extension.ref);

if (index !== -1)
{
list.splice(index, 1);
}
}
);
},
};

Expand Down
139 changes: 139 additions & 0 deletions packages/core/test/extensions.tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import type { ExtensionMetadata, ExtensionType } from '@pixi/core';
import { extensions } from '@pixi/core';

const exampleType = 'test-extension' as ExtensionType;
const exampleType2 = 'test-extension2' as ExtensionType;

const example = {
extension: {
type: exampleType,
name: 'test',
} as ExtensionMetadata,
};

const example2 = {
extension: exampleType as ExtensionMetadata,
};

describe('extensions', () =>
{
afterEach(() =>
{
extensions['_addHandlers'][exampleType] = undefined;
extensions['_removeHandlers'][exampleType] = undefined;
extensions['_addHandlers'][exampleType2] = undefined;
extensions['_removeHandlers'][exampleType2] = undefined;
});

describe('handle', () =>
{
it('should throw when extension type is handled twice', () =>
{
extensions.handle(exampleType, () => null, () => null);
expect(() =>
{
extensions.handle(exampleType, () => null, () => null);
}).toThrowError(`Extension type ${exampleType} already has a handler`);
});
});

describe('handleByMap', () =>
{
it('should successfully handle an extension by a map', () =>
{
const map = {} as Record<string, any>;

extensions.handleByMap(exampleType, map);
extensions.add(example);
expect(map.test).toBe(example);
extensions.remove(example);
expect(map.test).toBeUndefined();
});
});

describe('handleByList', () =>
{
it('should successfully handle an extension by a map', () =>
{
const list: any[] = [];

extensions.handleByList(exampleType, list);
extensions.add(example);
expect(list[0]).toBe(example);
extensions.remove(example);
expect(list[0]).toBeUndefined();
});
});

describe('add', () =>
{
it('should register simple extension data', () =>
{
const list: any[] = [];

extensions.handleByList(exampleType, list);
extensions.add(example2);
expect(list.length).toBe(1);
expect(list[0]).toBe(example2);
extensions.remove(example2);
expect(list.length).toBe(0);
});

it('should support spread', () =>
{
const list: any[] = [];

extensions.handleByList(exampleType, list);
extensions.add(example2, example);
expect(list.length).toBe(2);
extensions.remove(example2, example);
expect(list.length).toBe(0);
});

it('should immedately register extension before handle', () =>
{
const list: any[] = [];

extensions.handleByList(exampleType, list);
extensions.add(example);
expect(list.length).toBe(1);
expect(list[0]).toBe(example);
extensions.remove(example);
expect(list.length).toBe(0);
});

it('should immedately register extension after handle', () =>
{
const list: any[] = [];

extensions.add(example);
extensions.handleByList(exampleType, list);
expect(list.length).toBe(1);
expect(list[0]).toBe(example);
extensions.remove(example);
expect(list.length).toBe(0);
});

it('should support multiple types', () =>
{
const list: any[] = [];
const list2: any[] = [];
const example3 = {
extension: {
type: [exampleType, exampleType2] as ExtensionType[],
} as ExtensionMetadata,
};

extensions.handleByList(exampleType, list);
extensions.handleByList(exampleType2, list2);
extensions.add(example3);
expect(list.length).toBe(1);
expect(list2.length).toBe(1);
expect(list[0]).toBe(example3);
expect(list2[0]).toBe(example3);
extensions.remove(example3);
expect(list.length).toBe(0);
expect(list2.length).toBe(0);
});
});
});
22 changes: 1 addition & 21 deletions packages/loaders/src/Loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,27 +648,7 @@ class Loader
}
}

extensions.handle(
ExtensionType.Loader,
(extension) =>
{
const plugin = extension.ref as unknown as ILoaderPlugin;

Loader._plugins.push(plugin);
plugin.add?.();
},
(extension) =>
{
const plugins = Loader._plugins;
const plugin = extension.ref as unknown as ILoaderPlugin;
const index = plugins.indexOf(plugin);

if (index !== -1)
{
plugins.splice(index, 1);
}
}
);
extensions.handleByList(ExtensionType.Loader, Loader._plugins);

Loader.prototype.add = function add(this: Loader, name: any, url?: any, options?: any, callback?: any): Loader
{
Expand Down

0 comments on commit 84096a7

Please sign in to comment.