Skip to content

Commit

Permalink
Re-enable custom element test and fix "undefined" child (#3095)
Browse files Browse the repository at this point in the history
* Re-enable custom element test and fix "undefined" child

* Remove outdated comment

* Adds a changeset
  • Loading branch information
matthewp committed Apr 13, 2022
1 parent 200a01c commit 5acf77d
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 42 deletions.
6 changes: 6 additions & 0 deletions .changeset/neat-oranges-tan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'astro': patch
'@astrojs/webapi': patch
---

Fixes rendering of "undefined" on custom element children
4 changes: 2 additions & 2 deletions packages/astro/src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,9 @@ function resolveFlags(flags: Partial<Flags>): 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,
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}</${Component}>`
: `>${children == null ? '' : children}</${Component}>`
)}`
);
}
Expand Down
31 changes: 12 additions & 19 deletions packages/astro/test/custom-elements.test.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions packages/astro/test/fixtures/custom-elements/astro.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { defineConfig } from 'astro/config';
import ceIntegration from '@test/custom-element-renderer';

export default defineConfig({
integrations: [ceIntegration()],
experimental: {
integrations: true
}
})
Original file line number Diff line number Diff line change
@@ -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(),
});
},
},
};
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import './shim.js';

function getConstructor(Component) {
if (typeof Component === 'string') {
const tagName = Component;
Expand Down
5 changes: 5 additions & 0 deletions packages/webapi/src/lib/CustomElementRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
15 changes: 15 additions & 0 deletions packages/webapi/src/lib/Element.ts
Original file line number Diff line number Diff line change
@@ -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

Expand Down

0 comments on commit 5acf77d

Please sign in to comment.