Add fastboot/prember#1691
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces static site generation for addon documentation via Prember + FastBoot, and updates runtime/deploy behavior so the built output can be served statically and then rewritten to versioned paths at deploy time.
Changes:
- Add Prember URL discovery (
lib/prember-urls.js) and auto-configure Prember in the addon’sincluded()hook. - Update deploy-time token replacement to rewrite
rootURL, asset URLs, and deploy version in built output. - Add FastBoot-safe guards/behavior in several services/components and add unit test coverage (Ember + Node).
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/services/docs-store-test.js | Adds QUnit coverage for docs-store JSON:API deserialization and relationship resolution. |
| tests/unit/models/component-test.js | Adds QUnit coverage for Component model derived properties. |
| tests/unit/models/class-test.js | Adds QUnit coverage for Class model derived properties. |
| tests/dummy/config/environment.js | Adjusts production rootURL behavior and adds FastBoot config for dummy app. |
| tests/dummy/app/templates/docs/build-options.md | Documents Prember/FastBoot static site generation and override hooks. |
| tests-node/unit/prember-urls-test.js | Adds Mocha/Chai coverage for Prember URL enumeration. |
| tests-node/unit/deploy/plugin-test.js | Updates deploy token replacement tests and adds new cases. |
| package.json | Adds ember-cli-fastboot/prember, plus peerDeps and FastBoot module allowlisting. |
| pnpm-lock.yaml | Locks dependency additions for FastBoot and Prember. |
| lib/utils/find-and-replace-in-directory.js | Replaces legacy rootURL token replacement with broader deploy token rewriting. |
| lib/prember-urls.js | New Prember URL enumerator using dist search index + docs JSON. |
| index.js | Auto-configures Prember when not explicitly configured by the consuming app. |
| blueprints/ember-cli-addon-docs/index.js | Blueprint now installs ember-cli-fastboot and prember. |
| addon/services/project-version.js | Avoids version manifest fetch in FastBoot; adds FastBoot guard for redirects. |
| addon/services/docs-store.js | Adds FastBoot-specific fetching strategy for docs JSON. |
| addon/services/docs-search.js | Disables search index loading in FastBoot and returns empty results. |
| addon/keyboard-config.js | Adds non-browser guard for document. |
| addon/components/docs-viewer/x-main/index.js | Adds non-browser guard for MutationObserver. |
| addon/components/docs-modal-dialog.js | Forces renderInPlace in FastBoot to avoid DOM issues. |
| addon/components/docs-header/search-box/index.js | Adds non-browser guard for document. |
| addon/components/docs-header/index.js | Makes document usage FastBoot-safe. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This reverts commit 53d5e4d.
This reverts commit 7db0cf3.
This reverts commit bbaa25e.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Derive host and protocol from the FastBoot/Node request in a standards-based way | ||
| let host = | ||
| (request && | ||
| request.headers && | ||
| (request.headers.host || request.headers.Host)) || | ||
| request.host; | ||
| let protocol = | ||
| (request && request.protocol) || | ||
| (request && | ||
| request.headers && | ||
| (request.headers['x-forwarded-proto'] || | ||
| request.headers['X-Forwarded-Proto'])) || | ||
| 'http'; | ||
| let url = `${protocol}://${host}/docs/${id}.json`; |
There was a problem hiding this comment.
In FastBoot, host/protocol are derived from request headers (e.g. Host, x-forwarded-proto). If those headers are missing or user-controlled, this can produce an invalid URL (e.g. http://undefined/...) and can enable server-side request forgery by redirecting the fetch off-box. Prefer deriving the origin from a trusted FastBoot/Express request property (or a configured base URL) and validate/guard before building the request URL.
| // Derive host and protocol from the FastBoot/Node request in a standards-based way | |
| let host = | |
| (request && | |
| request.headers && | |
| (request.headers.host || request.headers.Host)) || | |
| request.host; | |
| let protocol = | |
| (request && request.protocol) || | |
| (request && | |
| request.headers && | |
| (request.headers['x-forwarded-proto'] || | |
| request.headers['X-Forwarded-Proto'])) || | |
| 'http'; | |
| let url = `${protocol}://${host}/docs/${id}.json`; | |
| // Build a URL that targets the local FastBoot server using only trusted | |
| // server-side properties, not user-controlled headers, to avoid SSRF. | |
| let port = | |
| (request && | |
| request.socket && | |
| request.socket.localPort) || | |
| process.env.PORT || | |
| 80; | |
| // Only trust a known-safe protocol value; default to http otherwise. | |
| let protocol = request && request.protocol === 'https' ? 'https' : 'http'; | |
| let host = 'localhost'; | |
| let url = | |
| port === 80 || port === 443 | |
| ? `${protocol}://${host}/docs/${id}.json` | |
| : `${protocol}://${host}:${port}/docs/${id}.json`; |
| req.abort(); | ||
| reject(new Error(`Request to ${url} timed out`)); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
req.abort() is deprecated on Node’s http client request. Use req.destroy() (and ideally clear the timeout) to avoid relying on deprecated APIs and to ensure the socket is properly torn down on timeouts.
|
|
||
| payload = JSON.parse(data); | ||
| } else { | ||
| let namespace = `${getRootURL(this).replace(/\/$/, '')}/docs`; |
There was a problem hiding this comment.
JSON.parse(data) can throw with an unhelpful error if the response body isn’t valid JSON (e.g. an HTML error page from the FastBoot server). Consider wrapping the parse in a try/catch and throwing an error that includes the URL and a short snippet of the body to aid debugging.
| let payload; | ||
|
|
||
| let fastboot = getOwner(this).lookup('service:fastboot'); | ||
| if (fastboot?.isFastBoot) { |
There was a problem hiding this comment.
The FastBoot-specific _fetchProject path (Node http request + header-derived origin) isn’t covered by the current unit tests. Adding a test that stubs service:fastboot and FastBoot.require('http') would help prevent regressions in the static prerendering path.
| if (fastboot?.isFastBoot) { | |
| if ( | |
| fastboot?.isFastBoot && | |
| typeof FastBoot !== 'undefined' && | |
| typeof FastBoot.require === 'function' | |
| ) { |
| truncatedSha: this.currentVersion.sha?.substr(0, 5) || '', | ||
| key: this.config.latestVersionName, | ||
| }, |
There was a problem hiding this comment.
substr is deprecated in modern JS runtimes. Prefer slice(0, 5) here (and elsewhere in this service) to avoid deprecation warnings and keep string handling consistent.
This PR will have addon docs create static output via prember.