Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix namespaced component usage in MDX #4272

Merged
merged 9 commits into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/late-tips-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'astro': patch
'@astrojs/mdx': patch
---

Properly handle hydration for namespaced components
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from 'astro/config';
import preact from '@astrojs/preact';

// https://astro.build/config
export default defineConfig({
integrations: [preact()],
});
12 changes: 12 additions & 0 deletions packages/astro/e2e/fixtures/namespaced-component/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "@e2e/namespaced-component",
"version": "0.0.0",
"private": true,
"devDependencies": {
"@astrojs/preact": "workspace:*",
"astro": "workspace:*"
},
"dependencies": {
"preact": "^10.7.3"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { useState } from 'preact/hooks';

/** a counter written in Preact */
function PreactCounter({ children, id }) {
const [count, setCount] = useState(0);
const add = () => setCount((i) => i + 1);
const subtract = () => setCount((i) => i - 1);

return (
<div id={id} class="counter">
<button class="decrement" onClick={subtract}>-</button>
<pre>{count}</pre>
<button class="increment" onClick={add}>+</button>
<div class="children">{children}</div>
</div>
);
}

export const components = { PreactCounter }
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
import * as ns from '../components/PreactCounter.tsx';
---

<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width" />
<link rel="icon" type="image/x-icon" href="/favicon.ico" />
</head>
<body>
<main>
<ns.components.PreactCounter id="preact-counter" client:load>
<h1>preact</h1>
</ns.components.PreactCounter>
</main>
</body>
</html>
36 changes: 36 additions & 0 deletions packages/astro/e2e/namespaced-component.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { expect } from '@playwright/test';
import { testFactory } from './test-utils.js';

const test = testFactory({
root: './fixtures/namespaced-component/',
});

let devServer;

test.beforeEach(async ({ astro }) => {
devServer = await astro.startDevServer();
});

test.afterEach(async () => {
await devServer.stop();
});

test.describe('Hydrating namespaced components', () => {
test('Preact Component', async ({ page }) => {
await page.goto('/');

const counter = await page.locator('#preact-counter');
await expect(counter, 'component is visible').toBeVisible();

const count = await counter.locator('pre');
await expect(count, 'initial count is 0').toHaveText('0');

const children = await counter.locator('.children');
await expect(children, 'children exist').toHaveText('preact');

const increment = await counter.locator('.increment');
await increment.click();

await expect(count, 'count incremented by 1').toHaveText('1');
});
});
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"test:e2e:match": "playwright test -g"
},
"dependencies": {
"@astrojs/compiler": "^0.23.1",
"@astrojs/compiler": "^0.23.3",
"@astrojs/language-server": "^0.20.0",
"@astrojs/markdown-remark": "^1.0.0",
"@astrojs/telemetry": "^1.0.0",
Expand Down
72 changes: 71 additions & 1 deletion packages/astro/src/jsx/babel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function addClientMetadata(
}
if (!existingAttributes.find((attr) => attr === 'client:component-export')) {
if (meta.name === '*') {
meta.name = getTagName(node).split('.').at(1)!;
meta.name = getTagName(node).split('.').slice(1).join('.')!;
}
const componentExport = t.jsxAttribute(
t.jsxNamespacedName(t.jsxIdentifier('client'), t.jsxIdentifier('component-export')),
Expand Down Expand Up @@ -177,6 +177,76 @@ export default function astroJSX(): PluginObj {
}
state.set('imports', imports);
},
JSXMemberExpression(path, state) {
const node = path.node;
// Skip automatic `_components` in MDX files
if (state.filename?.endsWith('.mdx') && t.isJSXIdentifier(node.object) && node.object.name === '_components') {
return;
}
const parent = path.findParent((n) => t.isJSXElement(n))!;
const parentNode = parent.node as t.JSXElement;
const tagName = getTagName(parentNode);
if (!isComponent(tagName)) return;
if (!hasClientDirective(parentNode)) return;
const isClientOnly = isClientOnlyComponent(parentNode);
if (tagName === ClientOnlyPlaceholder) return;

const imports = state.get('imports') ?? new Map();
const namespace = tagName.split('.');
for (const [source, specs] of imports) {
for (const { imported, local } of specs) {
const reference = path.referencesImport(source, imported);
if (reference) {
path.setData('import', { name: imported, path: source });
break;
}
if (namespace.at(0) === local) {
path.setData('import', { name: imported, path: source });
break;
}
}
}

const meta = path.getData('import');
if (meta) {
let resolvedPath: string;
if (meta.path.startsWith('.')) {
const fileURL = pathToFileURL(state.filename!);
resolvedPath = `/@fs${new URL(meta.path, fileURL).pathname}`;
if (resolvedPath.endsWith('.jsx')) {
resolvedPath = resolvedPath.slice(0, -4);
}
} else {
resolvedPath = meta.path;
}

if (isClientOnly) {
(state.file.metadata as PluginMetadata).astro.clientOnlyComponents.push({
exportName: meta.name,
specifier: tagName,
resolvedPath,
});

meta.resolvedPath = resolvedPath;
addClientOnlyMetadata(parentNode, meta);
} else {
(state.file.metadata as PluginMetadata).astro.hydratedComponents.push({
exportName: '*',
specifier: tagName,
resolvedPath,
});

meta.resolvedPath = resolvedPath;
addClientMetadata(parentNode, meta);
}
} else {
throw new Error(
`Unable to match <${getTagName(
parentNode
)}> with client:* directive to an import statement!`
);
}
},
JSXIdentifier(path, state) {
const isAttr = path.findParent((n) => t.isJSXAttribute(n));
if (isAttr) return;
Expand Down
10 changes: 9 additions & 1 deletion packages/astro/src/runtime/server/astro-island.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,15 @@ declare const Astro: {
import(this.getAttribute('component-url')!),
rendererUrl ? import(rendererUrl) : () => () => {},
]);
this.Component = componentModule[this.getAttribute('component-export') || 'default'];
const componentExport = this.getAttribute('component-export') || 'default';
if (!componentExport.includes('.')) {
this.Component = componentModule[componentExport];
} else {
this.Component = componentModule;
for (const part of componentExport.split('.')) {
this.Component = this.Component[part]
}
}
this.hydrator = hydrator;
return this.hydrate;
},
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/runtime/server/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ export async function generateHydrateScript(
const { renderer, result, astroId, props, attrs } = scriptOptions;
const { hydrate, componentUrl, componentExport } = metadata;

if (!componentExport) {
if (!componentExport.value) {
throw new Error(
`Unable to resolve a componentExport for "${metadata.displayName}"! Please open an issue.`
`Unable to resolve a valid export for "${metadata.displayName}"! Please open an issue at https://astro.build/issues!`
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import mdx from '@astrojs/mdx';
import react from '@astrojs/react';

export default {
integrations: [react(), mdx()]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/mdx-namespace",
"dependencies": {
"astro": "workspace:*",
"@astrojs/mdx": "workspace:*",
"@astrojs/react": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const Component = () => {
return <p id="component">Hello world</p>;
};
export const ns = {
Component
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as mod from '../components/Component.jsx';

<mod.ns.Component client:load />
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { ns } from '../components/Component.jsx';

<ns.Component client:load />
83 changes: 83 additions & 0 deletions packages/integrations/mdx/test/mdx-namespace.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { expect } from 'chai';
import { parseHTML } from 'linkedom';
import { loadFixture } from '../../../astro/test/test-utils.js';

describe('MDX Namespace', () => {
let fixture;

before(async () => {
fixture = await loadFixture({
root: new URL('./fixtures/mdx-namespace/', import.meta.url),
});
});

describe('build', () => {
before(async () => {
await fixture.build();
});

it('works for object', async () => {
const html = await fixture.readFile('/object/index.html');
const { document } = parseHTML(html);

const island = document.querySelector('astro-island');
const component = document.querySelector('#component');

expect(island).not.undefined;
expect(component.textContent).equal('Hello world')
});

it('works for star', async () => {
const html = await fixture.readFile('/star/index.html');
const { document } = parseHTML(html);

const island = document.querySelector('astro-island');
const component = document.querySelector('#component');

expect(island).not.undefined;
expect(component.textContent).equal('Hello world')
});
});

describe('dev', () => {
let devServer;

before(async () => {
devServer = await fixture.startDevServer();
});

after(async () => {
await devServer.stop();
});

it('works for object', async () => {
const res = await fixture.fetch('/object');

expect(res.status).to.equal(200);

const html = await res.text();
const { document } = parseHTML(html);

const island = document.querySelector('astro-island');
const component = document.querySelector('#component');

expect(island).not.undefined;
expect(component.textContent).equal('Hello world')
});

it('works for star', async () => {
const res = await fixture.fetch('/star');

expect(res.status).to.equal(200);

const html = await res.text();
const { document } = parseHTML(html);

const island = document.querySelector('astro-island');
const component = document.querySelector('#component');

expect(island).not.undefined;
expect(component.textContent).equal('Hello world')
});
});
});
Loading