From 4db463c6bbcc3a17ee8eb591bea8e357120ecb35 Mon Sep 17 00:00:00 2001 From: Nick Perez Date: Tue, 14 May 2024 14:45:03 +0200 Subject: [PATCH] fix(core): configure should use the parent of the current instance, to avoid duplication (#5147) --- packages/core/src/Extension.ts | 1 + .../integration/core/extendExtensions.spec.ts | 221 ++++++++++++++++++ .../integration/core/extensionOptions.spec.ts | 51 ++++ 3 files changed, 273 insertions(+) diff --git a/packages/core/src/Extension.ts b/packages/core/src/Extension.ts index 225c9299536..8bb371240a7 100644 --- a/packages/core/src/Extension.ts +++ b/packages/core/src/Extension.ts @@ -457,6 +457,7 @@ export class Extension { // with different calls of `configure` const extension = this.extend() + extension.parent = this.parent extension.options = mergeDeep(this.options as Record, options) as Options extension.storage = callOrReturn( diff --git a/tests/cypress/integration/core/extendExtensions.spec.ts b/tests/cypress/integration/core/extendExtensions.spec.ts index bb586777d0c..89103508adc 100644 --- a/tests/cypress/integration/core/extendExtensions.spec.ts +++ b/tests/cypress/integration/core/extendExtensions.spec.ts @@ -43,6 +43,30 @@ describe('extend extensions', () => { }) }) + it('should have a parent', () => { + const extension = Extension + .create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + + const newExtension = extension + .extend({ + addAttributes() { + return { + bar: {}, + } + }, + }) + + const parent = newExtension.parent + + expect(parent).to.eq(extension) + }) + it('should merge configs', () => { const extension = Extension .create({ @@ -104,6 +128,40 @@ describe('extend extensions', () => { }) }) + it('should set parents multiple times', () => { + const grandparentExtension = Extension + .create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + + const parentExtension = grandparentExtension + .extend({ + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + const childExtension = parentExtension + .extend({ + addAttributes() { + return { + ...this.parent?.(), + baz: {}, + } + }, + }) + + expect(parentExtension.parent).to.eq(grandparentExtension) + expect(childExtension.parent).to.eq(parentExtension) + }) + it('should merge configs without direct parent configuration', () => { const extension = Extension .create({ @@ -130,4 +188,167 @@ describe('extend extensions', () => { bar: {}, }) }) + + it('should call ancestors only once', () => { + const callCounts = { + grandparent: 0, + parent: 0, + child: 0, + } + + const extension = Extension + .create({ + addAttributes() { + callCounts.grandparent += 1 + return { + foo: {}, + } + }, + }) + .extend({ + addAttributes() { + callCounts.parent += 1 + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + .extend({ + addAttributes() { + callCounts.child += 1 + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + getExtensionField(extension, 'addAttributes')() + + expect(callCounts).to.deep.eq({ + grandparent: 1, + parent: 1, + child: 1, + }) + }) + + it('should call ancestors only once on configure', () => { + const callCounts = { + grandparent: 0, + parent: 0, + child: 0, + } + + const extension = Extension + .create({ + addAttributes() { + callCounts.grandparent += 1 + return { + foo: {}, + } + }, + }) + .extend({ + addAttributes() { + callCounts.parent += 1 + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + .extend({ + addAttributes() { + callCounts.child += 1 + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + .configure({ + baz: {}, + }) + + getExtensionField(extension, 'addAttributes')() + + expect(callCounts).to.deep.eq({ + grandparent: 1, + parent: 1, + child: 1, + }) + }) + + it('should use grandparent as parent on configure (not parent)', () => { + const grandparentExtension = Extension + .create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + + const parentExtension = grandparentExtension + .extend({ + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + const childExtension = parentExtension + .configure({ + baz: {}, + }) + + expect(parentExtension.parent).to.eq(grandparentExtension) + expect(childExtension.parent).to.eq(grandparentExtension) + }) + + it('should use parent\'s config on `configure`', () => { + const grandparentExtension = Extension + .create({ + name: 'grandparent', + addAttributes() { + return { + foo: {}, + } + }, + }) + + const parentExtension = grandparentExtension + .extend({ + name: 'parent', + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + const childExtension = parentExtension + .configure({ + baz: {}, + }) + + expect(childExtension.config.name).to.eq('parent') + }) + + it('should inherit config on configure', () => { + + const parentExtension = Extension + .create({ + name: 'did-inherit', + }) + + const childExtension = parentExtension + .configure() + + expect(childExtension.config.name).to.eq('did-inherit') + }) }) diff --git a/tests/cypress/integration/core/extensionOptions.spec.ts b/tests/cypress/integration/core/extensionOptions.spec.ts index 7bce58c9238..5773daeb820 100644 --- a/tests/cypress/integration/core/extensionOptions.spec.ts +++ b/tests/cypress/integration/core/extensionOptions.spec.ts @@ -84,6 +84,40 @@ describe('extension options', () => { }) }) + it('should be extendable multiple times', () => { + const extension = Extension.create({ + addOptions() { + return { + foo: 1, + bar: 1, + } + }, + }).extend({ + addOptions() { + return { + ...this.parent?.(), + baz: 1, + } + }, + }) + + const newExtension = extension.extend({ + addOptions() { + return { + ...this.parent?.(), + bax: 1, + } + }, + }) + + expect(newExtension.options).to.deep.eq({ + foo: 1, + bar: 1, + baz: 1, + bax: 1, + }) + }) + it('should be overwritable', () => { const extension = Extension .create({ @@ -138,6 +172,23 @@ describe('extension options', () => { }) }) + it('should configure retaining existing config', () => { + const extension = Extension.create({ + name: 'parent', + addOptions() { + return { + foo: 1, + bar: 1, + } + }, + }) + + const newExtension = extension + .configure() + + expect(newExtension.config.name).to.eq('parent') + }) + it('should create its own instance on configure', () => { const extension = Extension .create({