-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Extract Pages functions build logic into its own package #12027
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
base: main
Are you sure you want to change the base?
Conversation
Extracts the Pages functions-to-worker compilation logic from wrangler into a standalone package. This is the first step towards Autoconfig Pages (DEVX-2407).
🦋 Changeset detectedLatest commit: 22dfdee The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
- Resolve path-to-regexp path at codegen time using require.resolve() - Wrangler/esbuild bundles it when building the final worker - Remove esbuild dependency (no longer needed for bundling) - Return 404 when no route matches and fallback binding unavailable - Add vitest-pool-workers runtime tests (9 tests covering route matching, middleware execution, and 404 handling)
5b356a3 to
99322a1
Compare
Manual test fixture for the @cloudflare/pages-functions package. Demonstrates compiling a functions directory and running with wrangler.
99322a1 to
e437fac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "extends": "@cloudflare/workers-tsconfig", | ||
| "compilerOptions": { | ||
| "module": "NodeNext", | ||
| "moduleResolution": "NodeNext", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any reason why this is NodeNext?
(this seems to me quite inconsistent with the monorepo where Bundler is generally used)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried switching to bundler, but had to change back so the CLI would work without bundling.
| import { | ||
| consolidateRoutes, | ||
| MAX_FUNCTIONS_ROUTES_RULES, | ||
| } from "./routes-consolidation.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the .js imports are, I believe, very inconsistent with the monorepo (because NodeNext is used as the moduleResolution) is this intentional?
- Change compileFunctions() to accept project directory instead of functions directory - Add functionsDir option (default: 'functions') - Add pages-functions CLI for command-line compilation - Update README with CLI documentation and accurate API reference - Restructure test fixtures to match project-based API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "undefined", | ||
| ]; | ||
|
|
||
| const reservedKeywordRegex = new RegExp(`^${RESERVED_KEYWORDS.join("|")}$`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Incorrect regex construction causes valid identifiers to be rejected
The reservedKeywordRegex in identifiers.ts is incorrectly constructed using ^${RESERVED_KEYWORDS.join("|")}$ which produces the pattern ^do|if|in|for|...|undefined$. Due to regex operator precedence, the ^ anchor only applies to the first alternative ("do") and the $ anchor only applies to the last alternative ("undefined"). The middle alternatives match anywhere in the string.
This causes identifiers like "doSomething", "forLoop", or "infinity" to incorrectly match as reserved keywords:
const reservedKeywordRegex = new RegExp(`^${RESERVED_KEYWORDS.join("|"))}$`);
reservedKeywordRegex.test("doSomething"); // true (should be false)
reservedKeywordRegex.test("forLoop"); // true (should be false)The fix should wrap the alternation in a group: `^(${RESERVED_KEYWORDS.join("|")})
While this bug is copied from the existing wrangler code (packages/wrangler/src/pages/functions/identifiers.ts:55), it's being added as new code in this package. In practice, the bug has limited impact because the code only processes exports matching onRequest* pattern, which don't start with reserved keywords. However, any future use of isValidIdentifier() with identifiers starting with short reserved keywords would fail incorrectly.
Recommendation: Change line 59 to: const reservedKeywordRegex = new RegExp(\^(
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes https://jira.cfdata.org/browse/DEVX-2407
Extracts the Pages functions-to-worker compilation logic from wrangler into a standalone
@cloudflare/pages-functionspackage.This is part of the Autoconfig Pages work (RM-26054). The goal is to enable converting Pages projects with a functions directory into Workers projects that can be deployed with the standard
wrangler deployflow.What this package does
Takes a functions directory:
And compiles it to a worker entrypoint that can be bundled and deployed like any regular worker:
What's extracted
_routes.jsongeneration)What stays in wrangler
bundleWorker)assets:import pluginpages functions build, etc.)Test coverage
vitest-pool-workersthat test actual request handling in the workers runtime (route matching, middleware execution, 404 handling)Next steps
A follow-up PR will wire this into wrangler's
buildFunctions()to replace the duplicated code.