Skip to content

fix(pfe-tools): use WDS plugin for TS redirect instead of koa middleware#3144

Open
bennypowers wants to merge 2 commits into
staging/pfv6from
fix/pfe-tools-ts-redirect
Open

fix(pfe-tools): use WDS plugin for TS redirect instead of koa middleware#3144
bennypowers wants to merge 2 commits into
staging/pfv6from
fix/pfe-tools-ts-redirect

Conversation

@bennypowers
Copy link
Copy Markdown
Member

Summary

  • Convert liveReloadTsChangesMiddleware from koa middleware to a WDS serve hook plugin
  • The middleware ran AFTER koa-send (static file serving), so when a .js file didn't exist on disk, koa-send returned 404 before the middleware could rewrite the path to .ts
  • As a WDS plugin with serve hook, the path rewrite runs BEFORE static file resolution
  • Fixes 404 errors for elements with only .ts source files (no pre-compiled .js), such as pf-v6-tooltip

Test plan

  • npx wtr elements/pf-v5-clipboard-copy/test/pf-clipboard-copy.spec.ts passes with no compiled .js in elements/
  • Full test suite: 820 passed, 1 failed (pre-existing select test), 31 skipped
  • No import errors for pf-v6-tooltip or other elements

The liveReloadTsChangesMiddleware ran as koa middleware, which executes
AFTER WDS static file serving (koa-send). When a .js file didn't exist
on disk, koa-send returned 404 before the middleware could redirect to
the .ts source.

Convert to a WDS plugin with a serve hook, which runs BEFORE static
file resolution. This fixes 404 errors for elements that only have .ts
source files (no pre-compiled .js).

Assisted-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 12, 2026

⚠️ No Changeset found

Latest commit: c0b44f9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

✅ Commitlint tests passed!

More Info
{
  "valid": true,
  "errors": [],
  "warnings": [],
  "input": "fix(pfe-tools): use WDS plugin for TS redirect instead of koa middleware"
}

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit e8a44ab
😎 Deploy Preview https://deploy-preview-3144--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions Bot added the AT passed Automated testing has passed label May 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

SSR Test Run for 7d94b15: Report

The previous approach (serve hook) broke esbuild compilation because
returning from serve() prevents other plugins from running. Instead,
keep the koa middleware but call next() first, then redirect to .ts
only when the .js file doesn't exist (404). This is backwards
compatible: when compiled .js exists, it serves normally.

Assisted-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

SSR Test Run for e8a44ab: Report

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the PFE dev-server request handling to improve TypeScript-source resolution when .js files are requested but only .ts sources exist in the repo, reducing 404s during local development and test runs.

Changes:

  • Adjusts liveReloadTsChangesMiddleware to run after downstream middleware and only redirect when the response is a 404.
  • Removes the inline comment describing the previous regex capture-group behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 91 to 97
const TYPESCRIPT_SOURCES_RE = new RegExp(`(${config.elementsDir}|pfe-core)/.*\\.js`);

return function(ctx, next) {
if (!ctx.path.includes('node_modules') && ctx.path
.match(TYPESCRIPT_SOURCES_RE)) {
return async function(ctx, next) {
await next();
if (ctx.status === 404
&& !ctx.path.includes('node_modules')
&& TYPESCRIPT_SOURCES_RE.test(ctx.path)) {
ctx.redirect(ctx.path.replace('.js', '.ts'));
Comment on lines +92 to 98
return async function(ctx, next) {
await next();
if (ctx.status === 404
&& !ctx.path.includes('node_modules')
&& TYPESCRIPT_SOURCES_RE.test(ctx.path)) {
ctx.redirect(ctx.path.replace('.js', '.ts'));
} else {
return next();
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AT passed Automated testing has passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants