From 5acf77dd22be95e33ff838383a2c1790f484e380 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Wed, 13 Apr 2022 08:44:22 -0400 Subject: [PATCH] Re-enable custom element test and fix "undefined" child (#3095) * Re-enable custom element test and fix "undefined" child * Remove outdated comment * Adds a changeset --- .changeset/neat-oranges-tan.md | 6 +++ packages/astro/src/core/config.ts | 4 +- packages/astro/src/runtime/server/index.ts | 2 +- packages/astro/test/custom-elements.test.js | 31 +++++------- .../fixtures/custom-elements/astro.config.mjs | 9 ++++ .../custom-elements/my-component-lib/index.js | 49 ++++++++++++------- .../my-component-lib/server.js | 2 - .../webapi/src/lib/CustomElementRegistry.ts | 5 ++ packages/webapi/src/lib/Element.ts | 15 ++++++ 9 files changed, 81 insertions(+), 42 deletions(-) create mode 100644 .changeset/neat-oranges-tan.md create mode 100644 packages/astro/test/fixtures/custom-elements/astro.config.mjs diff --git a/.changeset/neat-oranges-tan.md b/.changeset/neat-oranges-tan.md new file mode 100644 index 000000000..e059c04f1 --- /dev/null +++ b/.changeset/neat-oranges-tan.md @@ -0,0 +1,6 @@ +--- +'astro': patch +'@astrojs/webapi': patch +--- + +Fixes rendering of "undefined" on custom element children diff --git a/packages/astro/src/core/config.ts b/packages/astro/src/core/config.ts index e127131c5..e4c517632 100644 --- a/packages/astro/src/core/config.ts +++ b/packages/astro/src/core/config.ts @@ -329,9 +329,9 @@ function resolveFlags(flags: Partial): CLIFlags { config: typeof flags.config === 'string' ? flags.config : undefined, host: typeof flags.host === 'string' || typeof flags.host === 'boolean' ? flags.host : undefined, - experimentalSsr: typeof flags.experimentalSsr === 'boolean' ? flags.experimentalSsr : false, + experimentalSsr: typeof flags.experimentalSsr === 'boolean' ? flags.experimentalSsr : undefined, experimentalIntegrations: - typeof flags.experimentalIntegrations === 'boolean' ? flags.experimentalIntegrations : false, + typeof flags.experimentalIntegrations === 'boolean' ? flags.experimentalIntegrations : undefined, drafts: typeof flags.drafts === 'boolean' ? flags.drafts : false, }; } diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts index 067468497..84047025f 100644 --- a/packages/astro/src/runtime/server/index.ts +++ b/packages/astro/src/runtime/server/index.ts @@ -290,7 +290,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr await render`<${Component}${spreadAttributes(props)}${markHTMLString( (children == null || children == '') && voidElementNames.test(Component) ? `/>` - : `>${children}` + : `>${children == null ? '' : children}` )}` ); } diff --git a/packages/astro/test/custom-elements.test.js b/packages/astro/test/custom-elements.test.js index a34efca14..cecf72d63 100644 --- a/packages/astro/test/custom-elements.test.js +++ b/packages/astro/test/custom-elements.test.js @@ -1,25 +1,23 @@ import { expect } from 'chai'; -import cheerio from 'cheerio'; +import { load as cheerioLoad } from 'cheerio'; import { loadFixture } from './test-utils.js'; -// TODO(fks): This seemed to test a custom renderer, but it seemed to be a copy -// fixture of lit. Should this be moved into a publicly published integration now, -// and then tested as an example? Or, should we just remove. Skipping now -// to tackle later, since our lit tests cover similar code paths. -describe.skip('Custom Elements', () => { +describe('Custom Elements', () => { let fixture; before(async () => { fixture = await loadFixture({ root: './fixtures/custom-elements/', - intergrations: ['@test/custom-element-renderer'], + experimental: { + integrations: true + } }); await fixture.build(); }); it('Work as constructors', async () => { const html = await fixture.readFile('/ctr/index.html'); - const $ = cheerio.load(html); + const $ = cheerioLoad(html); // test 1: Element rendered expect($('my-element')).to.have.lengthOf(1); @@ -30,7 +28,7 @@ describe.skip('Custom Elements', () => { it('Works with exported tagName', async () => { const html = await fixture.readFile('/index.html'); - const $ = cheerio.load(html); + const $ = cheerioLoad(html); // test 1: Element rendered expect($('my-element')).to.have.lengthOf(1); @@ -41,7 +39,7 @@ describe.skip('Custom Elements', () => { it('Hydration works with exported tagName', async () => { const html = await fixture.readFile('/load/index.html'); - const $ = cheerio.load(html); + const $ = cheerioLoad(html); // SSR // test 1: Element rendered @@ -52,27 +50,22 @@ describe.skip('Custom Elements', () => { // Hydration // test 3: Component and polyfill scripts bundled separately - expect($('script[type=module]')).to.have.lengthOf(2); - }); - - it('Polyfills are added even if not hydrating', async () => { - const html = await fixture.readFile('/index.html'); - const $ = cheerio.load(html); - expect($('script[type=module]')).to.have.lengthOf(1); }); it('Custom elements not claimed by renderer are rendered as regular HTML', async () => { const html = await fixture.readFile('/nossr/index.html'); - const $ = cheerio.load(html); + const $ = cheerioLoad(html); // test 1: Rendered the client-only element expect($('client-element')).to.have.lengthOf(1); + // No children + expect($('client-element').text()).to.equal(''); }); it('Can import a client-only element that is nested in JSX', async () => { const html = await fixture.readFile('/nested/index.html'); - const $ = cheerio.load(html); + const $ = cheerioLoad(html); // test 1: Element rendered expect($('client-only-element')).to.have.lengthOf(1); diff --git a/packages/astro/test/fixtures/custom-elements/astro.config.mjs b/packages/astro/test/fixtures/custom-elements/astro.config.mjs new file mode 100644 index 000000000..4c426a9e7 --- /dev/null +++ b/packages/astro/test/fixtures/custom-elements/astro.config.mjs @@ -0,0 +1,9 @@ +import { defineConfig } from 'astro/config'; +import ceIntegration from '@test/custom-element-renderer'; + +export default defineConfig({ + integrations: [ceIntegration()], + experimental: { + integrations: true + } +}) diff --git a/packages/astro/test/fixtures/custom-elements/my-component-lib/index.js b/packages/astro/test/fixtures/custom-elements/my-component-lib/index.js index b13bb52ec..a550dfee2 100644 --- a/packages/astro/test/fixtures/custom-elements/my-component-lib/index.js +++ b/packages/astro/test/fixtures/custom-elements/my-component-lib/index.js @@ -1,18 +1,31 @@ -export default { - name: '@test/custom-element-renderer', - server: './server.js', - polyfills: [ - './polyfill.js' - ], - hydrationPolyfills: [ - './hydration-polyfill.js' - ], - viteConfig() { - return { - optimizeDeps: { - include: ['@test/custom-element-renderer/polyfill.js', '@test/custom-element-renderer/hydration-polyfill.js'], - exclude: ['@test/custom-element-renderer/server.js'] - } - } - } -}; +function getViteConfiguration() { + return { + optimizeDeps: { + include: ['@test/custom-element-renderer/polyfill.js', '@test/custom-element-renderer/hydration-polyfill.js'], + exclude: ['@test/custom-element-renderer/server.js'] + }, + }; +} + +export default function () { + return { + name: '@test/custom-element-renderer', + hooks: { + 'astro:config:setup': ({ updateConfig, addRenderer, injectScript }) => { + // Inject the necessary polyfills on every page + injectScript('head-inline', `import '@test/custom-element-renderer/polyfill.js';`); + // Inject the hydration code, before a component is hydrated. + injectScript('before-hydration', `import '@test/custom-element-renderer/hydration-polyfill.js';`); + // Add the lit renderer so that Astro can understand lit components. + addRenderer({ + name: '@test/custom-element-renderer', + serverEntrypoint: '@test/custom-element-renderer/server.js', + }); + // Update the vite configuration. + updateConfig({ + vite: getViteConfiguration(), + }); + }, + }, + }; +} diff --git a/packages/astro/test/fixtures/custom-elements/my-component-lib/server.js b/packages/astro/test/fixtures/custom-elements/my-component-lib/server.js index 32466e4ed..7e36a26fd 100644 --- a/packages/astro/test/fixtures/custom-elements/my-component-lib/server.js +++ b/packages/astro/test/fixtures/custom-elements/my-component-lib/server.js @@ -1,5 +1,3 @@ -import './shim.js'; - function getConstructor(Component) { if (typeof Component === 'string') { const tagName = Component; diff --git a/packages/webapi/src/lib/CustomElementRegistry.ts b/packages/webapi/src/lib/CustomElementRegistry.ts index 3f7020087..3429e1e73 100644 --- a/packages/webapi/src/lib/CustomElementRegistry.ts +++ b/packages/webapi/src/lib/CustomElementRegistry.ts @@ -26,6 +26,11 @@ export class CustomElementRegistry { if (!/-/.test(name)) throw new SyntaxError('Custom element name must contain a hyphen') + _.INTERNALS.set(constructor, { + attributes: {}, + localName: name, + } as any) + internals.constructorByName.set(name, constructor) internals.nameByConstructor.set(constructor, name) diff --git a/packages/webapi/src/lib/Element.ts b/packages/webapi/src/lib/Element.ts index 5827a0be0..1553f2863 100644 --- a/packages/webapi/src/lib/Element.ts +++ b/packages/webapi/src/lib/Element.ts @@ -1,6 +1,21 @@ import * as _ from './utils' export class Element extends Node { + constructor() { + super(); + + if(_.INTERNALS.has(new.target)) { + const internals = _.internalsOf(new.target, 'Element', 'localName') + _.INTERNALS.set(this, { + attributes: {}, + localName: internals.localName, + ownerDocument: this.ownerDocument, + shadowInit: null as unknown as ShadowRootInit, + shadowRoot: null as unknown as ShadowRoot, + } as ElementInternals) + } + } + hasAttribute(name: string): boolean { void name