Skip to content

Add the template mechanism, template hooks, and generatedFrom - #122

Closed
harmony7 wants to merge 1 commit into
feat/v8.1/02-generate-appfrom
feat/v8.1/03-templates
Closed

harmony7 wants to merge 1 commit into
feat/v8.1/02-generate-appfrom
feat/v8.1/03-templates

Conversation

@harmony7

Copy link
Copy Markdown
Member

Part 3 of 13 of the 8.1 stack (PR 3, item 3). Base: PR 2.

Changes

  • The app's src/index.js and its dependencies now come from a template (AppTemplate, in scaffold/templates/). The current app is the plain template, the only one in 8.1. Users cannot select a template yet (that comes in 8.2). A template only adds files: generateApp() throws for a template file that generateApp() writes itself.
  • Every generated app has the npm scripts prebuild, postbuild, and postinstall. Each one runs compute-js-static-publish run-template-hook <name>, which runs scripts/template_<name>.js if it exists. The CLI handles run-template-hook before the banner, so it prints nothing extra.
  • static-publish.rc.js of a new app has generatedFrom: the template, the version of this package, and the date.
  • README: correct the name of the storage content directory.

Review points

  • The golden files change only by the three scripts and generatedFrom. src/index.js is the same.

Tests

  • Unit tests for templates, and end-to-end tests for the hooks.
  • npm test on this branch: 120 passed.

Docs

CHANGELOG, README, and src/cli/CLAUDE.md.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@harmony7
harmony7 requested review from a team and kpfleming and removed request for a team October 27, 2026 06:52
@harmony7
harmony7 marked this pull request as ready for review October 27, 2026 06:52

// Runs scripts/template_<name>.js of the app in the current directory, if it
// exists. The npm scripts prebuild, postbuild, and postinstall of every
// generated app call this, so a template (or the user, in an ejected app) can

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is an 'ejected app'?

Comment on lines +12 to +37
/// <reference types="@fastly/js-compute" />
import { env } from 'fastly:env';
import { PublisherServer } from '@fastly/compute-js-static-publish';
import rc from '../static-publish.rc.js';
const publisherServer = PublisherServer.fromStaticPublishRc(rc);

// eslint-disable-next-line no-restricted-globals
addEventListener("fetch", (event) => event.respondWith(handleRequest(event)));
async function handleRequest(event) {

console.log('FASTLY_SERVICE_VERSION', env('FASTLY_SERVICE_VERSION'));

const request = event.request;

const response = await publisherServer.serveRequest(request);
if (response != null) {
return response;
}

// Do custom things here!
// Handle API requests, serve non-static responses, etc.

return new Response('Not found', { status: 404 });
}
`,
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not thrilled about this code being in a multiline string. Our tools won't be able to do type checking, linting, formatting, etc. If possible it would be much better for the files map to map target names to names of source files in the template's directory so they can be standalone files.

A similar concern exists for templates adding dependencies; if those aren't specified in a package.json then updating them will be a manual process.

@harmony7

Copy link
Copy Markdown
Member Author

We close this PR. We will split the 8.1 work again into fewer and larger PRs, to make the review easier. A new PR will replace this PR. The branch stays.

Thank you for the review. The new plan includes your feedback: the template files will be real files, not strings in the code.

@harmony7 harmony7 closed this Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants