From 67e9f4c3acd6789f07aafd27e1f9b9bd36bf8db7 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 18 Jun 2024 05:47:30 +0200 Subject: [PATCH 1/3] fix: apply #5147 fix to marks and nodes --- packages/core/src/Mark.ts | 1 + packages/core/src/Node.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/core/src/Mark.ts b/packages/core/src/Mark.ts index 1b0c46d2bbf..9ec52977412 100644 --- a/packages/core/src/Mark.ts +++ b/packages/core/src/Mark.ts @@ -591,6 +591,7 @@ export class Mark { // 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/packages/core/src/Node.ts b/packages/core/src/Node.ts index 7f3f02d35bc..b1c8c8f44af 100644 --- a/packages/core/src/Node.ts +++ b/packages/core/src/Node.ts @@ -782,6 +782,7 @@ export class Node { // 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( From daa3c6615334f35383925db43a0fe9ff971e4389 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 18 Jun 2024 05:43:56 +0200 Subject: [PATCH 2/3] fix: resolve Issue #4704 by reverting PR #4191 --- packages/core/src/Extension.ts | 22 +++---- packages/core/src/Mark.ts | 24 +++---- packages/core/src/Node.ts | 24 +++---- .../integration/core/extendExtensions.spec.ts | 65 +++++++++++++++++++ 4 files changed, 100 insertions(+), 35 deletions(-) diff --git a/packages/core/src/Extension.ts b/packages/core/src/Extension.ts index 966121318ab..1bb06428a22 100644 --- a/packages/core/src/Extension.ts +++ b/packages/core/src/Extension.ts @@ -457,17 +457,17 @@ export class Extension { configure(options: Partial = {}) { // return a new instance so we can use the same extension // with different calls of `configure` - const extension = this.extend() - + const extension = this.extend({ + ...this.config, + addOptions() { + return mergeDeep(this.parent?.() || {}, options) as Options + }, + }) + + // Always preserve the current name + extension.name = this.name + // Set the parent to be our parent extension.parent = this.parent - extension.options = mergeDeep(this.options as Record, options) as Options - - extension.storage = callOrReturn( - getExtensionField(extension, 'addStorage', { - name: extension.name, - options: extension.options, - }), - ) return extension } @@ -483,7 +483,7 @@ export class Extension { extension.name = extendedConfig.name ? extendedConfig.name : extension.parent.name - if (extendedConfig.defaultOptions) { + if (extendedConfig.defaultOptions && Object.keys(extendedConfig.defaultOptions).length > 0) { console.warn( `[tiptap warn]: BREAKING CHANGE: "defaultOptions" is deprecated. Please use "addOptions" instead. Found in extension: "${extension.name}".`, ) diff --git a/packages/core/src/Mark.ts b/packages/core/src/Mark.ts index 9ec52977412..9ff572b0faa 100644 --- a/packages/core/src/Mark.ts +++ b/packages/core/src/Mark.ts @@ -589,17 +589,17 @@ export class Mark { configure(options: Partial = {}) { // return a new instance so we can use the same extension // with different calls of `configure` - const extension = this.extend() - + const extension = this.extend({ + ...this.config, + addOptions() { + return mergeDeep(this.parent?.() || {}, options) as Options + }, + }) + + // Always preserve the current name + extension.name = this.name + // Set the parent to be our parent extension.parent = this.parent - extension.options = mergeDeep(this.options as Record, options) as Options - - extension.storage = callOrReturn( - getExtensionField(extension, 'addStorage', { - name: extension.name, - options: extension.options, - }), - ) return extension } @@ -607,7 +607,7 @@ export class Mark { extend( extendedConfig: Partial> = {}, ) { - const extension = new Mark({ ...this.config, ...extendedConfig }) + const extension = new Mark(extendedConfig) extension.parent = this @@ -615,7 +615,7 @@ export class Mark { extension.name = extendedConfig.name ? extendedConfig.name : extension.parent.name - if (extendedConfig.defaultOptions) { + if (extendedConfig.defaultOptions && Object.keys(extendedConfig.defaultOptions).length > 0) { console.warn( `[tiptap warn]: BREAKING CHANGE: "defaultOptions" is deprecated. Please use "addOptions" instead. Found in extension: "${extension.name}".`, ) diff --git a/packages/core/src/Node.ts b/packages/core/src/Node.ts index b1c8c8f44af..c1fd865f71c 100644 --- a/packages/core/src/Node.ts +++ b/packages/core/src/Node.ts @@ -780,17 +780,17 @@ export class Node { configure(options: Partial = {}) { // return a new instance so we can use the same extension // with different calls of `configure` - const extension = this.extend() - + const extension = this.extend({ + ...this.config, + addOptions() { + return mergeDeep(this.parent?.() || {}, options) as Options + }, + }) + + // Always preserve the current name + extension.name = this.name + // Set the parent to be our parent extension.parent = this.parent - extension.options = mergeDeep(this.options as Record, options) as Options - - extension.storage = callOrReturn( - getExtensionField(extension, 'addStorage', { - name: extension.name, - options: extension.options, - }), - ) return extension } @@ -798,7 +798,7 @@ export class Node { extend( extendedConfig: Partial> = {}, ) { - const extension = new Node({ ...this.config, ...extendedConfig }) + const extension = new Node(extendedConfig) extension.parent = this @@ -806,7 +806,7 @@ export class Node { extension.name = extendedConfig.name ? extendedConfig.name : extension.parent.name - if (extendedConfig.defaultOptions) { + if (extendedConfig.defaultOptions && Object.keys(extendedConfig.defaultOptions).length > 0) { console.warn( `[tiptap warn]: BREAKING CHANGE: "defaultOptions" is deprecated. Please use "addOptions" instead. Found in extension: "${extension.name}".`, ) diff --git a/tests/cypress/integration/core/extendExtensions.spec.ts b/tests/cypress/integration/core/extendExtensions.spec.ts index 89103508adc..415077aac0b 100644 --- a/tests/cypress/integration/core/extendExtensions.spec.ts +++ b/tests/cypress/integration/core/extendExtensions.spec.ts @@ -351,4 +351,69 @@ describe('extend extensions', () => { expect(childExtension.config.name).to.eq('did-inherit') }) + + it('should allow extending a configure', () => { + + const parentExtension = Extension + .create({ + name: 'did-inherit', + }) + + const childExtension = parentExtension + .configure().extend() + + expect(childExtension.config.name).to.eq('did-inherit') + }) + + it('should allow calling this.parent when extending a configure', () => { + + const parentExtension = Extension + .create({ + name: 'parentExtension', + addAttributes() { + return { + foo: {}, + } + }, + }) + + const childExtension = parentExtension + .configure({}).extend({ + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + const attributes = getExtensionField(childExtension, 'addAttributes')() + + expect(attributes).to.deep.eq({ + foo: {}, + bar: {}, + }) + }) + + it('should allow extending options when extending a configure', () => { + const parentExtension = Extension + .create({ + name: 'parentExtension', + addOptions() { + return { defaultOptions: 'ignored' } + }, + }) + + const childExtension = parentExtension + .configure({ configuredOptions: 'replace-default-options' }).extend({ + addOptions() { + return { ...this.parent?.(), additionalOptions: 'exist-too' } + }, + }) + + expect(childExtension.options).to.deep.eq({ + configuredOptions: 'replace-default-options', + additionalOptions: 'exist-too', + }) + }) }) From dee3946a4863cceafd1b85fc180262b6ebfd1909 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 18 Jun 2024 07:38:30 +0200 Subject: [PATCH 3/3] test: more robust tests for nodes and marks too --- .../integration/core/extendExtensions.spec.ts | 781 +++++++++--------- 1 file changed, 391 insertions(+), 390 deletions(-) diff --git a/tests/cypress/integration/core/extendExtensions.spec.ts b/tests/cypress/integration/core/extendExtensions.spec.ts index 415077aac0b..f57838a1929 100644 --- a/tests/cypress/integration/core/extendExtensions.spec.ts +++ b/tests/cypress/integration/core/extendExtensions.spec.ts @@ -1,419 +1,420 @@ /// -import { Extension, getExtensionField } from '@tiptap/core' +import { + Extension, getExtensionField, Mark, Node, +} from '@tiptap/core' describe('extend extensions', () => { - it('should define a config', () => { - const extension = Extension.create({ - addAttributes() { - return { + [Extension, Node, Mark].forEach(Extendable => { + describe(Extendable.create().type, () => { + it('should define a config', () => { + const extension = Extendable.create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + + const attributes = getExtensionField(extension, 'addAttributes')() + + expect(attributes).to.deep.eq({ foo: {}, - } - }, - }) - - const attributes = getExtensionField(extension, 'addAttributes')() - - expect(attributes).to.deep.eq({ - foo: {}, - }) - }) - - it('should overwrite a config', () => { - const extension = Extension - .create({ - addAttributes() { - return { - foo: {}, - } - }, - }) - .extend({ - addAttributes() { - return { - bar: {}, - } - }, - }) - - const attributes = getExtensionField(extension, 'addAttributes')() - - expect(attributes).to.deep.eq({ - bar: {}, - }) - }) - - 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({ - addAttributes() { - return { - foo: {}, - } - }, - }) - .extend({ - addAttributes() { - return { - ...this.parent?.(), - bar: {}, - } - }, + }) + }) + + it('should overwrite a config', () => { + const extension = Extendable + .create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + .extend({ + addAttributes() { + return { + bar: {}, + } + }, + }) + + const attributes = getExtensionField(extension, 'addAttributes')() + + expect(attributes).to.deep.eq({ + bar: {}, + }) + }) + + it('should have a parent', () => { + const extension = Extendable + .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 = Extendable + .create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + .extend({ + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + const attributes = getExtensionField(extension, 'addAttributes')() + + expect(attributes).to.deep.eq({ + foo: {}, + bar: {}, + }) + }) + + it('should merge configs multiple times', () => { + const extension = Extendable + .create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + .extend({ + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + .extend({ + addAttributes() { + return { + ...this.parent?.(), + baz: {}, + } + }, + }) + + const attributes = getExtensionField(extension, 'addAttributes')() + + expect(attributes).to.deep.eq({ + foo: {}, + bar: {}, + baz: {}, + }) + }) + + it('should set parents multiple times', () => { + const grandparentExtension = Extendable + .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 = Extendable + .create({ + addAttributes() { + return { + foo: {}, + } + }, + }) + .extend() + .extend({ + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + const attributes = getExtensionField(extension, 'addAttributes')() + + expect(attributes).to.deep.eq({ + foo: {}, + bar: {}, + }) }) - const attributes = getExtensionField(extension, 'addAttributes')() + it('should call ancestors only once', () => { + const callCounts = { + grandparent: 0, + parent: 0, + child: 0, + } - expect(attributes).to.deep.eq({ - foo: {}, - bar: {}, - }) - }) + const extension = Extendable + .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, + } - it('should merge configs multiple times', () => { - const extension = Extension - .create({ - addAttributes() { - return { - foo: {}, - } - }, - }) - .extend({ - addAttributes() { - return { - ...this.parent?.(), - bar: {}, - } - }, - }) - .extend({ - addAttributes() { - return { - ...this.parent?.(), + const extension = Extendable + .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: {}, - } - }, - }) - - const attributes = getExtensionField(extension, 'addAttributes')() - - expect(attributes).to.deep.eq({ - foo: {}, - bar: {}, - baz: {}, - }) - }) - - 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?.(), + }) + + 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 = Extendable + .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(parentExtension) - }) - - it('should merge configs without direct parent configuration', () => { - const extension = Extension - .create({ - addAttributes() { - return { - foo: {}, - } - }, - }) - .extend() - .extend({ - addAttributes() { - return { - ...this.parent?.(), - bar: {}, - } - }, - }) - - const attributes = getExtensionField(extension, 'addAttributes')() - - expect(attributes).to.deep.eq({ - foo: {}, - 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', () => { + }) + + expect(parentExtension.parent).to.eq(grandparentExtension) + expect(childExtension.parent).to.eq(grandparentExtension) + }) + + it('should use parent\'s config on `configure`', () => { + const grandparentExtension = Extendable + .create({ + name: 'grandparent', + addAttributes() { + return { + foo: {}, + } + }, + }) + + const parentExtension = grandparentExtension + .extend({ + name: 'parent', + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) + + const childExtension = parentExtension + .configure({ + baz: {}, + }) - const parentExtension = Extension - .create({ - name: 'did-inherit', + expect(childExtension.config.name).to.eq('parent') }) - const childExtension = parentExtension - .configure() - - expect(childExtension.config.name).to.eq('did-inherit') - }) - - it('should allow extending a configure', () => { + it('should allow extending a configure', () => { - const parentExtension = Extension - .create({ - name: 'did-inherit', - }) + const parentExtension = Extendable + .create({ - const childExtension = parentExtension - .configure().extend() + addAttributes() { + return { foo: 'bar' } + }, + }) - expect(childExtension.config.name).to.eq('did-inherit') - }) + const childExtension = parentExtension + .configure().extend() - it('should allow calling this.parent when extending a configure', () => { + const attributes = getExtensionField(childExtension, 'addAttributes')() - const parentExtension = Extension - .create({ - name: 'parentExtension', - addAttributes() { - return { - foo: {}, - } - }, + expect(attributes).to.deep.eq({ + foo: 'bar', + }) }) - const childExtension = parentExtension - .configure({}).extend({ - addAttributes() { - return { - ...this.parent?.(), - bar: {}, - } - }, - }) + it('should allow calling this.parent when extending a configure', () => { - const attributes = getExtensionField(childExtension, 'addAttributes')() + const parentExtension = Extendable + .create({ + name: 'parentExtension', + addAttributes() { + return { + foo: {}, + } + }, + }) - expect(attributes).to.deep.eq({ - foo: {}, - bar: {}, - }) - }) + const childExtension = parentExtension + .configure({}).extend({ + addAttributes() { + return { + ...this.parent?.(), + bar: {}, + } + }, + }) - it('should allow extending options when extending a configure', () => { - const parentExtension = Extension - .create({ - name: 'parentExtension', - addOptions() { - return { defaultOptions: 'ignored' } - }, - }) + const attributes = getExtensionField(childExtension, 'addAttributes')() - const childExtension = parentExtension - .configure({ configuredOptions: 'replace-default-options' }).extend({ - addOptions() { - return { ...this.parent?.(), additionalOptions: 'exist-too' } - }, + expect(attributes).to.deep.eq({ + foo: {}, + bar: {}, + }) + }) + + it('should deeply merge options when extending a configured extension', () => { + const parentExtension = Extendable + .create({ + name: 'parentExtension', + addOptions() { + return { defaultOptions: 'is-overwritten' } + }, + }) + + const childExtension = parentExtension + .configure({ configuredOptions: 'exists-too' }).extend({ + name: 'childExtension', + addOptions() { + return { ...this.parent?.(), additionalOptions: 'exist-too' } + }, + }) + + expect(childExtension.options).to.deep.eq({ + configuredOptions: 'exists-too', + additionalOptions: 'exist-too', + }) }) - - expect(childExtension.options).to.deep.eq({ - configuredOptions: 'replace-default-options', - additionalOptions: 'exist-too', }) }) })