From dc8357209e3d5de9d3ef09ae357ac2c6390b9e43 Mon Sep 17 00:00:00 2001 From: Vjacheslav Trushkin Date: Sat, 26 Feb 2022 12:13:50 +0200 Subject: [PATCH] Fix bugs in Vue components, causing them to render incorrectly with SSR --- packages/vue/src/iconify.ts | 30 +-- .../tests/api/20-rendering-from-api.test.js | 88 +++---- .../vue/tests/api/30-changing-props.test.js | 216 ++++++++++-------- packages/vue/tests/empty.js | 3 + packages/vue/tests/iconify/10-basic.test.js | 8 +- packages/vue/tests/iconify/10-empty.test.js | 31 +-- .../vue/tests/iconify/20-attributes.test.js | 41 +++- .../vue/tests/iconify/20-dimensions.test.js | 13 +- packages/vue/tests/iconify/20-ids.test.js | 9 +- packages/vue/tests/iconify/20-inline.test.js | 31 ++- .../tests/iconify/20-transformations.test.js | 49 +++- packages/vue2/.npmignore | 2 +- packages/vue2/__mocks__/vue/index.js | 6 + packages/vue2/src/iconify.ts | 38 ++- .../tests/api/20-rendering-from-api.test.js | 79 ++++--- .../vue2/tests/api/30-changing-props.test.js | 188 +++++++-------- packages/vue2/tests/empty.js | 3 + packages/vue2/tests/iconify/10-empty.test.js | 31 +-- 18 files changed, 465 insertions(+), 401 deletions(-) create mode 100644 packages/vue/tests/empty.js create mode 100644 packages/vue2/__mocks__/vue/index.js create mode 100644 packages/vue2/tests/empty.js diff --git a/packages/vue/src/iconify.ts b/packages/vue/src/iconify.ts index 0aefe0b..3f53965 100644 --- a/packages/vue/src/iconify.ts +++ b/packages/vue/src/iconify.ts @@ -230,6 +230,13 @@ if (typeof document !== 'undefined' && typeof window !== 'undefined') { } } +/** + * Empty icon data, rendered when icon is not available + */ +const emptyIcon = fullIcon({ + body: '', +}); + /** * Component */ @@ -246,14 +253,14 @@ export const Icon = defineComponent({ data() { return { // Mounted status - mounted: false, + iconMounted: false, // Callback counter to trigger re-render counter: 0, }; }, - beforeMount() { + mounted() { // Current icon name this._name = ''; @@ -261,7 +268,7 @@ export const Icon = defineComponent({ this._loadingIcon = null; // Mark as mounted - this.mounted = true; + this.iconMounted = true; }, unmounted() { @@ -275,6 +282,7 @@ export const Icon = defineComponent({ this._loadingIcon = null; } }, + // Get data for icon to render or null getIcon( icon: IconifyIcon | string, @@ -346,23 +354,19 @@ export const Icon = defineComponent({ // Render icon render() { - if (!this.mounted) { - return this.$slots.default ? this.$slots.default() : null; - } - // Re-render when counter changes this.counter; - // Get icon data const props = this.$attrs; - const icon: IconComponentData | null = this.getIcon( - props.icon, - props.onLoad - ); + + // Get icon data + const icon: IconComponentData | null = this.iconMounted + ? this.getIcon(props.icon, props.onLoad) + : null; // Validate icon object if (!icon) { - return this.$slots.default ? this.$slots.default() : null; + return render(emptyIcon, props); } // Add classes diff --git a/packages/vue/tests/api/20-rendering-from-api.test.js b/packages/vue/tests/api/20-rendering-from-api.test.js index b0b8cca..6b957df 100644 --- a/packages/vue/tests/api/20-rendering-from-api.test.js +++ b/packages/vue/tests/api/20-rendering-from-api.test.js @@ -1,10 +1,12 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon, loadIcons, iconExists } from '../../'; import { mockAPIData } from '@iconify/core/lib/api/modules/mock'; import { provider, nextPrefix } from './load'; +import { defaultIconResult } from '../empty'; const iconData = { body: '', @@ -63,19 +65,23 @@ describe('Rendering icon', () => { }, }; const wrapper = mount(Wrapper, {}); - const html = wrapper.html().replace(/\s*\n\s*/g, ''); - // Check HTML - expect(html).toEqual( - '' - ); + // Check HTML on next tick + nextTick() + .then(() => { + const html = wrapper.html().replace(/\s*\n\s*/g, ''); + expect(html).toEqual( + '' + ); - // Make sure onLoad has been called - expect(onLoadCalled).toEqual(true); + // Make sure onLoad has been called + expect(onLoadCalled).toEqual(true); - done(); + done(); + }) + .catch(done); }); }); @@ -108,24 +114,6 @@ describe('Rendering icon', () => { // Test it again expect(iconExists(iconName)).toEqual(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks - setTimeout(() => { - setTimeout(() => { - // Check HTML - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( - '' - ); - - // onLoad should have been called - expect(onLoadCalled).toEqual(true); - - done(); - }, 0); - }, 0); }, }); @@ -141,6 +129,22 @@ describe('Rendering icon', () => { expect(name).toEqual(iconName); expect(onLoadCalled).toEqual(false); onLoadCalled = true; + + // Test component on next tick + nextTick() + .then(() => { + // Check HTML + expect( + wrapper.html().replace(/\s*\n\s*/g, '') + ).toEqual( + '' + ); + + done(); + }) + .catch(done); }, }, data() { @@ -155,9 +159,6 @@ describe('Rendering icon', () => { }; const wrapper = mount(Wrapper, {}); - // Should render empty icon - expect(wrapper.html()).toEqual(''); - // onLoad should not have been called yet expect(onLoadCalled).toEqual(false); }); @@ -181,15 +182,21 @@ describe('Rendering icon', () => { // Test it again expect(iconExists(iconName)).toEqual(false); - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html()).toEqual(''); - + // Check if state was changed on next few ticks + nextTick() + .then(() => { + expect(wrapper.html()).toEqual(defaultIconResult); + return nextTick(); + }) + .then(() => { + expect(wrapper.html()).toEqual(defaultIconResult); + return nextTick(); + }) + .then(() => { + expect(wrapper.html()).toEqual(defaultIconResult); done(); - }, 0); - }, 0); + }) + .catch(done); }, }); @@ -207,8 +214,5 @@ describe('Rendering icon', () => { }, }; const wrapper = mount(Wrapper, {}); - - // Should render empty icon - expect(wrapper.html()).toEqual(''); }); }); diff --git a/packages/vue/tests/api/30-changing-props.test.js b/packages/vue/tests/api/30-changing-props.test.js index 868ae18..04081f9 100644 --- a/packages/vue/tests/api/30-changing-props.test.js +++ b/packages/vue/tests/api/30-changing-props.test.js @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon, iconExists } from '../../'; import { mockAPIData } from '@iconify/core/lib/api/modules/mock'; @@ -34,11 +35,45 @@ describe('Rendering icon', () => { // First onLoad call case iconName: expect(onLoadCalled).toEqual(''); + + // Wait 1 tick, then test rendered icon + nextTick() + .then(() => { + expect( + wrapper.html().replace(/\s*\n\s*/g, '') + ).toEqual( + '' + ); + + wrapper.setProps({ + icon: iconName2, + }); + }) + .catch(done); + break; // Second onLoad call case iconName2: expect(onLoadCalled).toEqual(iconName); + + // Wait 1 tick, then test rendered icon + nextTick() + .then(() => { + expect( + wrapper.html().replace(/\s*\n\s*/g, '') + ).toEqual( + '' + ); + + done(); + }) + .catch(done); + break; default: @@ -67,27 +102,8 @@ describe('Rendering icon', () => { // Send icon data next(); - // Test it again + // Make sure icon data is available expect(iconExists(iconName)).toEqual(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks (one to handle API response, one to re-render) - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( - '' - ); - - // onLoad should have been called - expect(onLoadCalled).toEqual(iconName); - - wrapper.setProps({ - icon: iconName2, - }); - }, 0); - }, 0); }, }); @@ -111,25 +127,8 @@ describe('Rendering icon', () => { // Send icon data next(); - // Test it again + // Make sure icon data is available expect(iconExists(iconName2)).toEqual(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( - '' - ); - - // onLoad should have been called for second icon - expect(onLoadCalled).toEqual(iconName2); - - done(); - }, 0); - }, 0); }, }); @@ -146,9 +145,6 @@ describe('Rendering icon', () => { }; const wrapper = mount(Wrapper, {}); - // Should render placeholder - expect(wrapper.html()).toEqual(''); - // onLoad should not have been called yet expect(onLoadCalled).toEqual(''); }); @@ -162,6 +158,35 @@ describe('Rendering icon', () => { const className = `iconify iconify--${prefix} iconify--${provider}`; let isSync = true; + const onLoad = (name) => { + switch (name) { + case iconName: + done('onLoad should not be called for initial icon'); + break; + + case iconName2: + // Wait 1 tick, then test rendered icon + nextTick() + .then(() => { + expect( + wrapper.html().replace(/\s*\n\s*/g, '') + ).toEqual( + '' + ); + + done(); + }) + .catch(done); + + break; + + default: + throw new Error(`Unexpected onLoad('${name}') call`); + } + }; + mockAPIData({ type: 'icons', provider, @@ -201,39 +226,24 @@ describe('Rendering icon', () => { // Send icon data next(); - // Test it again + // Make sure icon was loaded expect(iconExists(iconName2)).toEqual(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks (one to handle API response, one to re-render) - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( - '' - ); - - done(); - }, 0); - }, 0); }, }); // Check if icon has been loaded expect(iconExists(iconName)).toEqual(false); - // Render component // Render component const Wrapper = { components: { Icon }, - template: ``, + template: ``, + methods: { + onLoad, + }, }; const wrapper = mount(Wrapper, {}); - // Should render placeholder - expect(wrapper.html()).toEqual(''); - // Change icon name wrapper.setProps({ icon: iconName2, @@ -249,6 +259,46 @@ describe('Rendering icon', () => { const iconName = `@${provider}:${prefix}:${name}`; const className = `iconify iconify--${prefix} iconify--${provider}`; + const onLoad = (name) => { + expect(name).toBe(iconName); + + (async () => { + try { + // Wait 1 tick, test rendered icon + await nextTick(); + + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( + '' + ); + + // Add horizontal flip and style + wrapper.setProps({ + icon: iconName, + hFlip: true, + style: { + color: 'red', + }, + }); + + // Wait 1 tick + await nextTick(); + + // Test + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( + '' + ); + + done(); + } catch (err) { + done(err); + } + })(); + }; + mockAPIData({ type: 'icons', provider, @@ -266,42 +316,8 @@ describe('Rendering icon', () => { // Send icon data next(); - // Test it again + // Make sure icon was loaded expect(iconExists(iconName)).toEqual(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks (one to handle API response, one to re-render) - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( - '' - ); - - // Add horizontal flip and style - wrapper.setProps({ - icon: iconName, - hFlip: true, - style: { - color: 'red', - }, - }); - - // Wait for next tick - setTimeout(() => { - expect( - wrapper.html().replace(/\s*\n\s*/g, '') - ).toEqual( - '' - ); - - done(); - }, 0); - }, 0); - }, 0); }, }); @@ -311,11 +327,11 @@ describe('Rendering icon', () => { // Render component with placeholder text const Wrapper = { components: { Icon }, - template: `loading...`, + template: ``, + methods: { + onLoad, + }, }; const wrapper = mount(Wrapper, {}); - - // Should render placeholder - expect(wrapper.html()).toEqual('loading...'); }); }); diff --git a/packages/vue/tests/empty.js b/packages/vue/tests/empty.js new file mode 100644 index 0000000..a04d493 --- /dev/null +++ b/packages/vue/tests/empty.js @@ -0,0 +1,3 @@ +// Default data for empty icon +export const defaultIconResult = + ''; diff --git a/packages/vue/tests/iconify/10-basic.test.js b/packages/vue/tests/iconify/10-basic.test.js index 5f71751..90a7114 100644 --- a/packages/vue/tests/iconify/10-basic.test.js +++ b/packages/vue/tests/iconify/10-basic.test.js @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon } from '../../'; @@ -11,7 +12,7 @@ const iconData = { }; describe('Creating component', () => { - test('with wrapper', () => { + test('with wrapper', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -23,12 +24,14 @@ describe('Creating component', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( '' ); }); - test('without wrapper', () => { + test('without wrapper', async () => { const wrapper = mount(Icon, { props: { icon: iconData, @@ -38,6 +41,7 @@ describe('Creating component', () => { }, }, }); + await nextTick(); expect(wrapper.html().replace(/\s*\n\s*/g, '')).toEqual( '' diff --git a/packages/vue/tests/iconify/10-empty.test.js b/packages/vue/tests/iconify/10-empty.test.js index 101cadf..1b36c1f 100644 --- a/packages/vue/tests/iconify/10-empty.test.js +++ b/packages/vue/tests/iconify/10-empty.test.js @@ -3,6 +3,7 @@ */ import { mount } from '@vue/test-utils'; import { Icon } from '../../'; +import { defaultIconResult } from '../empty'; describe('Empty icon', () => { test('basic test', () => { @@ -10,40 +11,16 @@ describe('Empty icon', () => { props: {}, }); - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe(''); + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe(defaultIconResult); }); - test('with child node', () => { + test('with child node (child node is ignored)', () => { const Wrapper = { components: { Icon }, template: ``, }; const wrapper = mount(Wrapper, {}); - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - '' - ); - }); - - test('with text child node', () => { - const Wrapper = { - components: { Icon }, - template: `icon`, - }; - - const wrapper = mount(Wrapper, {}); - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe('icon'); - }); - - test('with multiple childen', () => { - const Wrapper = { - components: { Icon }, - template: `Home icon`, - }; - - const wrapper = mount(Wrapper, {}); - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - 'Home icon' - ); + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe(defaultIconResult); }); }); diff --git a/packages/vue/tests/iconify/20-attributes.test.js b/packages/vue/tests/iconify/20-attributes.test.js index f90eb6e..000a8c4 100644 --- a/packages/vue/tests/iconify/20-attributes.test.js +++ b/packages/vue/tests/iconify/20-attributes.test.js @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon } from '../../'; @@ -11,7 +12,7 @@ const iconData = { }; describe('Passing attributes', () => { - test('title', () => { + test('title', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -23,6 +24,8 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + const html = wrapper.html(); expect(html).toContain('role="img" title="Icon!"'); @@ -30,7 +33,7 @@ describe('Passing attributes', () => { expect(html).toContain('aria-hidden="true"'); }); - test('aria-hidden', () => { + test('aria-hidden', async () => { // dashes, string value const Wrapper = { components: { Icon }, @@ -43,10 +46,12 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).not.toContain('aria-hidden="true"'); }); - test('ariaHidden', () => { + test('ariaHidden', async () => { // camelCase, boolean value const Wrapper = { components: { Icon }, @@ -59,10 +64,12 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).not.toContain('aria-hidden="true"'); }); - test('style as string', () => { + test('style as string', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -74,10 +81,12 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('style="color: red;"'); }); - test('style as object', () => { + test('style as object', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -92,10 +101,12 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('style="color: red;"'); }); - test('color', () => { + test('color', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -107,10 +118,12 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('style="color: red;"'); }); - test('color with style', () => { + test('color with style', async () => { // color overrides style const Wrapper = { components: { Icon }, @@ -123,10 +136,12 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('style="color: purple;"'); }); - test('attributes that cannot change', () => { + test('attributes that cannot change', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -138,12 +153,14 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + const html = wrapper.html(); expect(html).not.toContain('viewBox="0 0 0 0"'); expect(html).not.toContain('preserveAspectRatio="none"'); }); - test('class', () => { + test('class', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -155,10 +172,12 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('class="test-icon"'); }); - test('class object', () => { + test('class object', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -171,6 +190,8 @@ describe('Passing attributes', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('class="active iconify"'); }); }); diff --git a/packages/vue/tests/iconify/20-dimensions.test.js b/packages/vue/tests/iconify/20-dimensions.test.js index 3cbd962..1d84e6f 100644 --- a/packages/vue/tests/iconify/20-dimensions.test.js +++ b/packages/vue/tests/iconify/20-dimensions.test.js @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon } from '../../'; @@ -11,7 +12,7 @@ const iconData = { }; describe('Dimensions', () => { - test('height', () => { + test('height', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -23,6 +24,8 @@ describe('Dimensions', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + const html = wrapper.html(); expect(html).toContain('height="48"'); expect(html).toContain('width="48"'); @@ -30,7 +33,7 @@ describe('Dimensions', () => { expect(html).not.toContain('width="1em"'); }); - test('width and height', () => { + test('width and height', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -42,6 +45,8 @@ describe('Dimensions', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + const html = wrapper.html(); expect(html).toContain('height="48"'); expect(html).toContain('width="32"'); @@ -49,7 +54,7 @@ describe('Dimensions', () => { expect(html).not.toContain('width="1em"'); }); - test('auto', () => { + test('auto', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -61,6 +66,8 @@ describe('Dimensions', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + const html = wrapper.html(); expect(html).toContain('height="24"'); expect(html).toContain('width="24"'); diff --git a/packages/vue/tests/iconify/20-ids.test.js b/packages/vue/tests/iconify/20-ids.test.js index b65b9cd..a1086b2 100644 --- a/packages/vue/tests/iconify/20-ids.test.js +++ b/packages/vue/tests/iconify/20-ids.test.js @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon } from '../../'; @@ -11,7 +12,7 @@ const iconDataWithID = { }; describe('Replacing IDs', () => { - test('default behavior', () => { + test('default behavior', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -23,10 +24,12 @@ describe('Replacing IDs', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).not.toContain('ssvg-id-1st-place-medala'); }); - test('custom generator', () => { + test('custom generator', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -38,6 +41,8 @@ describe('Replacing IDs', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('id="testID0"'); }); }); diff --git a/packages/vue/tests/iconify/20-inline.test.js b/packages/vue/tests/iconify/20-inline.test.js index 1779c63..70ba1f1 100644 --- a/packages/vue/tests/iconify/20-inline.test.js +++ b/packages/vue/tests/iconify/20-inline.test.js @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon } from '../../'; @@ -11,7 +12,7 @@ const iconData = { }; describe('Inline attribute', () => { - test('string', () => { + test('string', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -23,10 +24,12 @@ describe('Inline attribute', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('style="vertical-align: -0.125em;"'); }); - test('false string', () => { + test('false string', async () => { // "false" should be ignored const Wrapper = { components: { Icon }, @@ -39,12 +42,14 @@ describe('Inline attribute', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).not.toContain( 'style="vertical-align: -0.125em;"' ); }); - test('true', () => { + test('true', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -56,10 +61,12 @@ describe('Inline attribute', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('style="vertical-align: -0.125em;"'); }); - test('false', () => { + test('false', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -71,12 +78,14 @@ describe('Inline attribute', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).not.toContain( 'style="vertical-align: -0.125em;"' ); }); - test('inline and style string', () => { + test('inline and style string', async () => { // Style goes after vertical-align const Wrapper = { components: { Icon }, @@ -89,12 +98,14 @@ describe('Inline attribute', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain( 'color: red; vertical-align: -0.125em;' ); }); - test('inline and style object', () => { + test('inline and style object', async () => { // Style goes after vertical-align const Wrapper = { components: { Icon }, @@ -110,12 +121,14 @@ describe('Inline attribute', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain( 'color: red; vertical-align: -0.125em;' ); }); - test('inline and style overriding it', () => { + test('inline and style overriding it', async () => { // Style goes after vertical-align const Wrapper = { components: { Icon }, @@ -131,6 +144,8 @@ describe('Inline attribute', () => { }; const wrapper = mount(Wrapper, {}); - expect(wrapper.html()).toContain('style="vertical-align: 0;"'); + await nextTick(); + + expect(wrapper.html()).toContain('vertical-align: 0;"'); }); }); diff --git a/packages/vue/tests/iconify/20-transformations.test.js b/packages/vue/tests/iconify/20-transformations.test.js index 5de2139..64183ce 100644 --- a/packages/vue/tests/iconify/20-transformations.test.js +++ b/packages/vue/tests/iconify/20-transformations.test.js @@ -1,6 +1,7 @@ /** * @jest-environment jsdom */ +import { nextTick } from 'vue'; import { mount } from '@vue/test-utils'; import { Icon } from '../../'; @@ -11,7 +12,7 @@ const iconData = { }; describe('Rotation', () => { - test('number', () => { + test('number', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -23,12 +24,14 @@ describe('Rotation', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( '' ); }); - test('string', () => { + test('string', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -40,12 +43,14 @@ describe('Rotation', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain(''); }); }); describe('Flip', () => { - test('boolean', () => { + test('boolean', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -57,12 +62,14 @@ describe('Flip', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain( '' ); }); - test('string', () => { + test('string', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -74,12 +81,14 @@ describe('Flip', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain( '' ); }); - test('string and boolean', () => { + test('string and boolean', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -91,10 +100,12 @@ describe('Flip', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain(''); }); - test('string for boolean attribute', () => { + test('string for boolean attribute', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -106,12 +117,14 @@ describe('Flip', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain( '' ); }); - test('shorthand and boolean', () => { + test('shorthand and boolean', async () => { // 'flip' is processed after 'hFlip' because of order of elements in object, overwriting value const Wrapper = { components: { Icon }, @@ -124,12 +137,14 @@ describe('Flip', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain( '' ); }); - test('shorthand and boolean as string', () => { + test('shorthand and boolean as string', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -141,10 +156,12 @@ describe('Flip', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain(''); }); - test('wrong case', () => { + test('wrong case', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -156,12 +173,14 @@ describe('Flip', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).not.toContain('`, @@ -173,12 +192,14 @@ describe('Alignment and slice', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain( 'preserveAspectRatio="xMidYMin slice"' ); }); - test('string', () => { + test('string', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -190,10 +211,12 @@ describe('Alignment and slice', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('preserveAspectRatio="xMinYMax meet"'); }); - test('aliases', () => { + test('aliases', async () => { const Wrapper = { components: { Icon }, template: ``, @@ -205,6 +228,8 @@ describe('Alignment and slice', () => { }; const wrapper = mount(Wrapper, {}); + await nextTick(); + expect(wrapper.html()).toContain('preserveAspectRatio="xMinYMax meet"'); }); }); diff --git a/packages/vue2/.npmignore b/packages/vue2/.npmignore index 7637f49..7f03c2c 100644 --- a/packages/vue2/.npmignore +++ b/packages/vue2/.npmignore @@ -7,4 +7,4 @@ api-extractor.json build.js tsconfig.json *.config.js - +__mocks__ diff --git a/packages/vue2/__mocks__/vue/index.js b/packages/vue2/__mocks__/vue/index.js new file mode 100644 index 0000000..f61ff46 --- /dev/null +++ b/packages/vue2/__mocks__/vue/index.js @@ -0,0 +1,6 @@ +import Vue from 'vue'; + +Vue.config.productionTip = false; +Vue.config.devtools = false; + +export default Vue; diff --git a/packages/vue2/src/iconify.ts b/packages/vue2/src/iconify.ts index be91bc0..6609c03 100644 --- a/packages/vue2/src/iconify.ts +++ b/packages/vue2/src/iconify.ts @@ -219,6 +219,13 @@ if (typeof document !== 'undefined' && typeof window !== 'undefined') { } } +/** + * Empty icon data, rendered when icon is not available + */ + const emptyIcon = fullIcon({ + body: '', +}); + /** * Component */ @@ -236,7 +243,7 @@ export const Icon = Vue.extend({ data() { return { // Mounted status - mounted: false, + iconMounted: false, }; }, @@ -248,7 +255,7 @@ export const Icon = Vue.extend({ this._loadingIcon = null; // Mark as mounted - this.mounted = true; + this.iconMounted = true; }, beforeDestroy() { @@ -333,38 +340,21 @@ export const Icon = Vue.extend({ // Render icon render(createElement: CreateElement): VNode { - function placeholder(slots): VNode { - // Render child nodes - if (slots.default) { - const result = slots.default; - if (result instanceof Array && result.length > 0) { - // If there are multiple child nodes, they must be wrapped in Vue 2 - return result.length === 1 - ? result[0] - : createElement('span', result); - } - } - return null as unknown as VNode; - } - if (!this.mounted) { - return placeholder(this.$slots); - } + const props = this.$attrs; + let context = this.$data; // Get icon data - const props = this.$attrs; - const icon: IconComponentData | null = this.getIcon( + const icon: IconComponentData | null = this.iconMounted ? this.getIcon( props.icon, props.onLoad - ); + ) : null; // Validate icon object if (!icon) { - // Render child nodes - return placeholder(this.$slots); + return render(createElement, props, context, emptyIcon); } // Add classes - let context = this.$data; if (icon.classes) { context = { ...context, diff --git a/packages/vue2/tests/api/20-rendering-from-api.test.js b/packages/vue2/tests/api/20-rendering-from-api.test.js index da68c93..c41253b 100644 --- a/packages/vue2/tests/api/20-rendering-from-api.test.js +++ b/packages/vue2/tests/api/20-rendering-from-api.test.js @@ -1,10 +1,12 @@ /** * @jest-environment jsdom */ +import Vue from 'vue'; import { mount } from '@vue/test-utils'; import { Icon, loadIcons, iconExists } from '../../'; import { mockAPIData } from '@iconify/core/lib/api/modules/mock'; import { provider, nextPrefix } from './load'; +import { defaultIconResult } from '../empty'; const iconData = { body: @@ -66,17 +68,19 @@ describe('Rendering icon', () => { const wrapper = mount(Wrapper, {}); const html = wrapper.html().replace(/\s*\n\s*/g, ''); - // Check HTML - expect(html).toBe( - '' - ); + // Check HTML on next tick + Vue.nextTick(() => { + expect(html).toBe( + '' + ); - // Make sure onLoad has been called - expect(onLoadCalled).toBe(true); + // Make sure onLoad has been called + expect(onLoadCalled).toBe(true); - done(); + done(); + }); }); }); @@ -109,25 +113,6 @@ describe('Rendering icon', () => { // Test it again expect(iconExists(iconName)).toBe(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks - setTimeout(() => { - setTimeout(() => { - // Check HTML - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - '' - ); - - // onLoad should have been called - expect(onLoadCalled).toBe(true); - - done(); - }, 0); - }, 0); }, }); @@ -143,6 +128,22 @@ describe('Rendering icon', () => { expect(name).toBe(iconName); expect(onLoadCalled).toBe(false); onLoadCalled = true; + + // Test component on next tick + Vue.nextTick(() => { + // Check HTML + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( + '' + ); + + // onLoad should have been called + expect(onLoadCalled).toBe(true); + + done(); + }); }, }, data() { @@ -157,9 +158,6 @@ describe('Rendering icon', () => { }; const wrapper = mount(Wrapper, {}); - // Should render empty icon - expect(wrapper.html()).toBe(''); - // onLoad should not have been called yet expect(onLoadCalled).toBe(false); }); @@ -183,15 +181,16 @@ describe('Rendering icon', () => { // Test it again expect(iconExists(iconName)).toBe(false); - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html()).toBe(''); + // Check if state was changed after few ticks + Vue.nextTick(() => { + Vue.nextTick(() => { + Vue.nextTick(() => { + expect(wrapper.html()).toBe(defaultIconResult); - done(); - }, 0); - }, 0); + done(); + }); + }); + }); }, }); @@ -211,6 +210,6 @@ describe('Rendering icon', () => { const wrapper = mount(Wrapper, {}); // Should render empty icon - expect(wrapper.html()).toBe(''); + expect(wrapper.html()).toBe(defaultIconResult); }); }); diff --git a/packages/vue2/tests/api/30-changing-props.test.js b/packages/vue2/tests/api/30-changing-props.test.js index 4a1eb90..37ae50b 100644 --- a/packages/vue2/tests/api/30-changing-props.test.js +++ b/packages/vue2/tests/api/30-changing-props.test.js @@ -1,10 +1,12 @@ /** * @jest-environment jsdom */ +import Vue from 'vue'; import { mount } from '@vue/test-utils'; import { Icon, iconExists } from '../../'; import { mockAPIData } from '@iconify/core/lib/api/modules/mock'; import { provider, nextPrefix } from './load'; +import { defaultIconResult } from '../empty'; const iconData = { body: @@ -36,11 +38,41 @@ describe('Rendering icon', () => { // First onLoad call case iconName: expect(onLoadCalled).toBe(''); + + // Wait 1 tick, then test rendered icon + Vue.nextTick(() => { + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( + '' + ); + + // onLoad should have been called + expect(onLoadCalled).toBe(iconName); + + wrapper.setProps({ + icon: iconName2, + }); + }); break; // Second onLoad call case iconName2: expect(onLoadCalled).toBe(iconName); + + // Wait 1 tick, then test rendered icon + Vue.nextTick(() => { + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( + '' + ); + + // onLoad should have been called for second icon + expect(onLoadCalled).toBe(iconName2); + + done(); + }); break; default: @@ -69,27 +101,8 @@ describe('Rendering icon', () => { // Send icon data next(); - // Test it again + // Make sure icon data is available expect(iconExists(iconName)).toBe(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks (one to handle API response, one to re-render) - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - '' - ); - - // onLoad should have been called - expect(onLoadCalled).toBe(iconName); - - wrapper.setProps({ - icon: iconName2, - }); - }, 0); - }, 0); }, }); @@ -113,25 +126,8 @@ describe('Rendering icon', () => { // Send icon data next(); - // Test it again + // Make sure icon data is available expect(iconExists(iconName2)).toBe(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - '' - ); - - // onLoad should have been called for second icon - expect(onLoadCalled).toBe(iconName2); - - done(); - }, 0); - }, 0); }, }); @@ -147,7 +143,7 @@ describe('Rendering icon', () => { }); // Should render placeholder - expect(wrapper.html()).toBe(''); + expect(wrapper.html()).toBe(defaultIconResult); // onLoad should not have been called yet expect(onLoadCalled).toBe(''); @@ -162,6 +158,31 @@ describe('Rendering icon', () => { const className = `iconify iconify--${prefix} iconify--${provider}`; let isSync = true; + const onLoad = name => { + switch (name) { + case iconName: + done('onLoad should not be called for initial icon'); + break; + + case iconName2: + // Wait 1 tick, then test rendered icon + Vue.nextTick(() => { + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( + '' + ); + + done(); + }); + + break; + + default: + throw new Error(`Unexpected onLoad('${name}') call`); + } + }; + mockAPIData({ type: 'icons', provider, @@ -203,20 +224,6 @@ describe('Rendering icon', () => { // Test it again expect(iconExists(iconName2)).toBe(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks (one to handle API response, one to re-render) - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - '' - ); - - done(); - }, 0); - }, 0); }, }); @@ -227,11 +234,12 @@ describe('Rendering icon', () => { const wrapper = mount(Icon, { propsData: { icon: iconName, + onLoad, }, }); // Should render placeholder - expect(wrapper.html()).toBe(''); + expect(wrapper.html()).toBe(defaultIconResult); // Change icon name wrapper.setProps({ @@ -248,6 +256,39 @@ describe('Rendering icon', () => { const iconName = `@${provider}:${prefix}:${name}`; const className = `iconify iconify--${prefix} iconify--${provider}`; + const onLoad = name => { + expect(name).toBe(iconName); + + // Wait 1 tick, test rendered icon + Vue.nextTick(() => { + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( + '' + ); + + // Add horizontal flip and style + wrapper.setProps({ + icon: iconName, + hFlip: true, + // Vue 2 issue: changing style in unit test doesn't work, so changing color + // TODO: test changing style in live demo to see if its a unit test bug or component issue + color: 'red', + }); + + // Wait for 1 tick and test again + Vue.nextTick(() => { + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( + '' + ); + + done(); + }); + }); + }; + mockAPIData({ type: 'icons', provider, @@ -265,42 +306,8 @@ describe('Rendering icon', () => { // Send icon data next(); - // Test it again + // Make sure icon was loaded expect(iconExists(iconName)).toBe(true); - - // Check if state was changed - // Wrapped in double setTimeout() because re-render takes 2 ticks (one to handle API response, one to re-render) - setTimeout(() => { - setTimeout(() => { - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - '' - ); - - // Add horizontal flip and style - wrapper.setProps({ - icon: iconName, - hFlip: true, - // Vue 2 issue: changing style in unit test doesn't work, so changing color - // TODO: test changing style in live demo to see if its a unit test bug or component issue - color: 'red', - }); - - // Wait for 1 tick - setTimeout(() => { - expect( - wrapper.html().replace(/\s*\n\s*/g, '') - ).toBe( - '' - ); - - done(); - }, 0); - }, 0); - }, 0); }, }); @@ -311,10 +318,11 @@ describe('Rendering icon', () => { const wrapper = mount(Icon, { propsData: { icon: iconName, + onLoad, }, }); // Should be empty - expect(wrapper.html()).toBe(''); + expect(wrapper.html()).toBe(defaultIconResult); }); }); diff --git a/packages/vue2/tests/empty.js b/packages/vue2/tests/empty.js new file mode 100644 index 0000000..a04d493 --- /dev/null +++ b/packages/vue2/tests/empty.js @@ -0,0 +1,3 @@ +// Default data for empty icon +export const defaultIconResult = + ''; diff --git a/packages/vue2/tests/iconify/10-empty.test.js b/packages/vue2/tests/iconify/10-empty.test.js index 77e2616..75e030c 100644 --- a/packages/vue2/tests/iconify/10-empty.test.js +++ b/packages/vue2/tests/iconify/10-empty.test.js @@ -3,6 +3,7 @@ */ import { mount } from '@vue/test-utils'; import { Icon } from '../../'; +import { defaultIconResult } from '../empty'; describe('Empty icon', () => { test('basic test', () => { @@ -10,40 +11,16 @@ describe('Empty icon', () => { propsData: {}, }); - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe(''); + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe(defaultIconResult); }); - test('with child node', () => { + test('with child node (child node is ignored)', () => { const Wrapper = { components: { Icon }, template: ``, }; const wrapper = mount(Wrapper, {}); - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - '' - ); - }); - - test('with text child node', () => { - const Wrapper = { - components: { Icon }, - template: `icon`, - }; - - const wrapper = mount(Wrapper, {}); - expect(wrapper.text()).toBe('icon'); - }); - - test('with multiple childen', () => { - const Wrapper = { - components: { Icon }, - template: ` Home icon`, - }; - - const wrapper = mount(Wrapper, {}); - expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe( - ' Home icon' - ); + expect(wrapper.html().replace(/\s*\n\s*/g, '')).toBe(defaultIconResult); }); });