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

bug/issue 1223 handle incorrect nested SSR page bundle output name mapping #1224

Merged
merged 4 commits into from
May 4, 2024
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
29 changes: 21 additions & 8 deletions packages/cli/src/config/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,23 +271,36 @@
};
}

// TODO could we use this instead?

Check warning on line 274 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO could we use this instead?'

Check warning on line 274 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO could we use this instead?'

Check warning on line 274 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO could we use this instead?'

Check warning on line 274 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO could we use this instead?'
// https://github.com/rollup/rollup/blob/v2.79.1/docs/05-plugin-development.md#resolveimportmeta
// https://github.com/ProjectEvergreen/greenwood/issues/1087
function greenwoodPatchSsrPagesEntryPointRuntimeImport() {
function greenwoodPatchSsrPagesEntryPointRuntimeImport(compilation) {
return {
name: 'greenwood-patch-ssr-pages-entry-point-runtime-import',
generateBundle(options, bundle) {
const { pagesDir, scratchDir } = compilation.context;

Object.keys(bundle).forEach((key) => {
if (key.startsWith('__')) {
// ___GWD_ENTRY_FILE_URL=${filename}___
// map rollup bundle names back to original SSR pages for output bundles and paths
if (key.startsWith('_')) {
const needle = bundle[key].code.match(/___GWD_ENTRY_FILE_URL=(.*.)___/);
if (needle) {

// handle windows shenanigans for facadeModuleId and path separators
if (new URL(`file://${bundle[key].facadeModuleId}`).pathname.startsWith(scratchDir.pathname) && needle) {
const entryPathMatch = needle[1];

bundle[key].code = bundle[key].code.replace(/'___GWD_ENTRY_FILE_URL=(.*.)___'/, `new URL('./_${entryPathMatch}', import.meta.url)`);
} else {
console.warn(`Could not find entry path match for bundle => ${key}`);
Object.keys(bundle).forEach((_) => {
// handle windows shenanigans for facadeModuleId and path separators
if (new URL(`file://${bundle[_].facadeModuleId}`).pathname === `${pagesDir.pathname}${entryPathMatch}`) {
bundle[key].code = bundle[key].code.replace(/'___GWD_ENTRY_FILE_URL=(.*.)___'/, `new URL('./${bundle[_].fileName}', import.meta.url)`);

compilation.graph.forEach((page, idx) => {
if (page.relativeWorkspacePagePath === `/${entryPathMatch}`) {
compilation.graph[idx].outputPath = key;
}
});
}
});
}
}
});
Expand Down Expand Up @@ -363,7 +376,7 @@
recursive: true
});

// TODO should routes and APIs have chunks?

Check warning on line 379 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO should routes and APIs have chunks?'

Check warning on line 379 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO should routes and APIs have chunks?'

Check warning on line 379 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO should routes and APIs have chunks?'

Check warning on line 379 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO should routes and APIs have chunks?'
// https://github.com/ProjectEvergreen/greenwood/issues/1118
return [{
input,
Expand All @@ -385,7 +398,7 @@
const getRollupConfigForSsr = async (compilation, input) => {
const { outputDir } = compilation.context;

// TODO should routes and APIs have chunks?

Check warning on line 401 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO should routes and APIs have chunks?'

Check warning on line 401 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO should routes and APIs have chunks?'

Check warning on line 401 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO should routes and APIs have chunks?'

Check warning on line 401 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO should routes and APIs have chunks?'
// https://github.com/ProjectEvergreen/greenwood/issues/1118
return [{
input,
Expand All @@ -405,7 +418,7 @@
}),
commonjs(),
greenwoodImportMetaUrl(compilation),
greenwoodPatchSsrPagesEntryPointRuntimeImport() // TODO a little hacky but works for now
greenwoodPatchSsrPagesEntryPointRuntimeImport(compilation) // TODO a little hacky but works for now
],
onwarn: (errorObj) => {
const { code, message } = errorObj;
Expand Down
15 changes: 11 additions & 4 deletions packages/cli/src/lifecycles/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,10 @@ async function bundleSsrPages(compilation) {

for (const page of compilation.graph) {
if (page.isSSR && !page.prerender) {
const { filename, imports, route, template, title } = page;
const entryFileUrl = new URL(`./_${filename}`, scratchDir);
const moduleUrl = new URL(`./${filename}`, pagesDir);
const { imports, route, template, title, relativeWorkspacePagePath } = page;
const entryFileUrl = new URL(`./_${relativeWorkspacePagePath.replace('/', '')}`, scratchDir);
const moduleUrl = new URL(`./${relativeWorkspacePagePath.replace('/', '')}`, pagesDir);
const outputPathRootUrl = new URL(`file://${path.dirname(entryFileUrl.pathname)}`);
const request = new Request(moduleUrl); // TODO not really sure how to best no-op this?
// TODO getTemplate has to be static (for now?)
// https://github.com/ProjectEvergreen/greenwood/issues/955
Expand All @@ -205,14 +206,20 @@ async function bundleSsrPages(compilation) {
staticHtml = await (await htmlOptimizer.optimize(new URL(`http://localhost:8080${route}`), new Response(staticHtml))).text();
staticHtml = staticHtml.replace(/[`\\$]/g, '\\$&'); // https://stackoverflow.com/a/75688937/417806

if (!await checkResourceExists(outputPathRootUrl)) {
await fs.mkdir(outputPathRootUrl, {
recursive: true
});
}

// better way to write out this inline code?
await fs.writeFile(entryFileUrl, `
import { executeRouteModule } from '${normalizePathnameForWindows(executeModuleUrl)}';

export async function handler(request) {
const compilation = JSON.parse('${JSON.stringify(compilation)}');
const page = JSON.parse('${JSON.stringify(page)}');
const moduleUrl = '___GWD_ENTRY_FILE_URL=${filename}___';
const moduleUrl = '___GWD_ENTRY_FILE_URL=${relativeWorkspacePagePath.replace('/', '')}___';
const data = await executeRouteModule({ moduleUrl, compilation, page, request });
let staticHtml = \`${staticHtml}\`;

Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/lifecycles/graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ const generateGraph = async (compilation) => {
* data: custom page frontmatter
* filename: base filename of the page
* id: filename without the extension
* relativeWorkspacePagePath: the file path relative to the user's workspace directory
* label: "pretty" text representation of the filename
* imports: per page JS or CSS file imports to be included in HTML output from frontmatter
* resources: sum of all resources for the entire page
Expand All @@ -206,6 +207,7 @@ const generateGraph = async (compilation) => {
data: customData || {},
filename,
id,
relativeWorkspacePagePath: relativePagePath,
label: id.split('-')
.map((idPart) => {
return `${idPart.charAt(0).toUpperCase()}${idPart.substring(1)}`;
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/lifecycles/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ async function getHybridServer(compilation) {
const request = transformKoaRequestIntoStandardRequest(url, ctx.request);

if (!config.prerender && matchingRoute.isSSR && !matchingRoute.prerender) {
const { handler } = await import(new URL(`./__${matchingRoute.filename}`, outputDir));
const { handler } = await import(new URL(`./${matchingRoute.outputPath}`, outputDir));
const response = await handler(request, compilation);

ctx.body = Readable.from(response.body);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/plugins/resource/plugin-standard-html.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class StandardHtmlResource extends ResourceInterface {
}

if (matchingRoute.isSSR) {
const routeModuleLocationUrl = new URL(`./${matchingRoute.filename}`, pagesDir);
const routeModuleLocationUrl = new URL(`.${matchingRoute.relativeWorkspacePagePath}`, pagesDir);
const routeWorkerUrl = this.compilation.config.plugins.find(plugin => plugin.type === 'renderer').provider().executeModuleUrl;

await new Promise(async (resolve, reject) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
import fs from 'fs/promises';
import os from 'os';
import { spawnSync } from 'child_process';

const packageJson = JSON.parse(await fs.readFile(new URL('./package.json', import.meta.url), 'utf-8'));
const myThemePackPlugin = () => [{
const myThemePackPlugin = (options = {}) => [{
type: 'context',
name: 'my-theme-pack:context',
provider: () => {
const { name } = packageJson;
const command = os.platform() === 'win32' ? 'npm.cmd' : 'npm';
const ls = spawnSync(command, ['ls', name]);

const isInstalled = ls.stdout.toString().indexOf('(empty)') < 0;
const templateLocation = isInstalled
? new URL(`./node_modules/${name}/dist/layouts/`, import.meta.url)
: new URL('./fixtures/layouts/', import.meta.url);
const templateLocation = options.__isDevelopment // eslint-disable-line no-underscore-dangle
? new URL('./fixtures/layouts/', import.meta.url)
: new URL(`./node_modules/${name}/dist/layouts/`, import.meta.url);

return {
templates: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ class MyThemePackDevelopmentResource extends ResourceInterface {

export default {
plugins: [
...myThemePackPlugin(),
...myThemePackPlugin({
__isDevelopment: true
}),
{
type: 'resource',
name: 'my-theme-pack:resource',
Expand Down
91 changes: 91 additions & 0 deletions packages/cli/test/cases/develop.ssr/develop.ssr.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
* components/
* footer.js
* pages/
* blog
* first-post.js
* index.js
* artists.js
* post.js
* templates/
Expand Down Expand Up @@ -284,6 +287,94 @@ describe('Develop Greenwood With: ', function() {
expect(paragraph[0].textContent).to.not.be.undefined;
});
});

describe('Develop command with HTML route response using default export and nested SSR Blog Index page', function() {
let response = {};
let dom = {};
let body;

before(async function() {
response = await fetch(`${hostname}/blog/`);
body = await response.clone().text();
dom = new JSDOM(body);
});

it('should return a 200 status', function(done) {
expect(response.status).to.equal(200);
done();
});

it('should return the correct content type', function(done) {
expect(response.headers.get('content-type')).to.equal('text/html');
done();
});

it('should return a response body', function(done) {
expect(body).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should have the expected postId as an <h1> tag in the body', function() {
const heading = dom.window.document.querySelectorAll('body > h1');

expect(heading.length).to.equal(1);
expect(heading[0].textContent).to.equal('Nested SSR page should work!');
});
});

describe('Develop command with HTML route response using default export and nested SSR Blog First Post page', function() {
let response = {};
let dom = {};
let body;

before(async function() {
response = await fetch(`${hostname}/blog/first-post/`);
body = await response.clone().text();
dom = new JSDOM(body);
});

it('should return a 200 status', function(done) {
expect(response.status).to.equal(200);
done();
});

it('should return the correct content type', function(done) {
expect(response.headers.get('content-type')).to.equal('text/html');
done();
});

it('should return a response body', function(done) {
expect(body).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should have the expected postId as an <h1> tag in the body', function() {
const heading = dom.window.document.querySelectorAll('body > h1');

expect(heading.length).to.equal(1);
expect(heading[0].textContent).to.equal('Nested SSR First Post page should work!');
});
});
});

after(function() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default class BlogFirstPostPage extends HTMLElement {
connectedCallback() {
this.innerHTML = `
<h1>Nested SSR First Post page should work!</h1>
`;
}
}
7 changes: 7 additions & 0 deletions packages/cli/test/cases/develop.ssr/src/pages/blog/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default class BlogPage extends HTMLElement {
connectedCallback() {
this.innerHTML = `
<h1>Nested SSR page should work!</h1>
`;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
* pages/
* about.md
* artists.js
* blog/
* first-post.js
* index.js
* index.js
* post.js
* users.js
Expand Down Expand Up @@ -272,6 +275,94 @@ describe('Serve Greenwood With: ', function() {
});
});

describe('Serve command with HTML route response using default export and nested SSR Blog Index page', function() {
let response = {};
let dom = {};
let body;

before(async function() {
response = await fetch(`${hostname}/blog/`);
body = await response.clone().text();
dom = new JSDOM(body);
});

it('should return a 200 status', function(done) {
expect(response.status).to.equal(200);
done();
});

it('should return the correct content type', function(done) {
expect(response.headers.get('content-type')).to.equal('text/html');
done();
});

it('should return a response body', function(done) {
expect(body).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should have the expected postId as an <h1> tag in the body', function() {
const heading = dom.window.document.querySelectorAll('body > h1');

expect(heading.length).to.equal(1);
expect(heading[0].textContent).to.equal('Nested SSR page should work!');
});
});

describe('Develop command with HTML route response using default export and nested SSR Blog First Post page', function() {
let response = {};
let dom = {};
let body;

before(async function() {
response = await fetch(`${hostname}/blog/first-post/`);
body = await response.clone().text();
dom = new JSDOM(body);
});

it('should return a 200 status', function(done) {
expect(response.status).to.equal(200);
done();
});

it('should return the correct content type', function(done) {
expect(response.headers.get('content-type')).to.equal('text/html');
done();
});

it('should return a response body', function(done) {
expect(body).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should be valid HTML from JSDOM', function(done) {
expect(dom).to.not.be.undefined;
done();
});

it('should have the expected postId as an <h1> tag in the body', function() {
const heading = dom.window.document.querySelectorAll('body > h1');

expect(heading.length).to.equal(1);
expect(heading[0].textContent).to.equal('Nested SSR First Post page should work!');
});
});

describe('Bundled image using new URL and import.meta.url', function() {
const bundledName = 'assets/logo-abb2e884.svg';
let response = {};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default class BlogFirstPostPage extends HTMLElement {
connectedCallback() {
this.innerHTML = `
<h1>Nested SSR First Post page should work!</h1>
`;
}
}
Loading
Loading