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

Pages Functions Compiler #160

Merged
merged 29 commits into from Jan 5, 2022
Merged

Pages Functions Compiler #160

merged 29 commits into from Jan 5, 2022

Conversation

GregBrimble
Copy link
Member

This PR open-sources our functions compiler which compiles a folder of functions into a single Worker script (previously this was done with the @cloudflare/pages-functions-compiler package).

It also adds support for sourcemaps (as requested in #138) and automatically watches all dependents (fixes #49).

Still to follow in a subsequent PR is a command which performs just this compile step once (which will be used by production builds). I'm thinking of having two modes:

  1. wrangler functions compile which builds functions will a fallback of fetch
  2. wrangler functions compile --service=SOME_SERVICE which builds functions with a fallback of env.SOME_SERVICE.fetch.

So Pages things would use wrangler functions compile --service=ASSETS.

This small piece of extra work would allow any user to use functions folder syntax to generate a Worker ✨

@GregBrimble GregBrimble added the pages Relating to Pages label Dec 22, 2021
@GregBrimble GregBrimble self-assigned this Dec 22, 2021
@changeset-bot
Copy link

changeset-bot bot commented Dec 22, 2021

🦋 Changeset detected

Latest commit: 8049f84

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

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

@threepointone
Copy link
Contributor

I'll review in a bit, but can start by saying this should probably be under pages, i.e- wrangler pages functions ...

@GregBrimble
Copy link
Member Author

To clarify, I haven't added any new commands yet. This PR is entirely built into wrangler pages dev. I was just talking about additionally exposing it in a different command in the future. I'm relatively indifferent to that being wrangler pages functions ... vs. wrangler functions .... Only thing I would say is that if people are using it outwith a Pages project, it's maybe a bit unexpected to use wrangler pages.

Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

dropped some notes on an initial skim, happy to discuss on a chat if you'd like

packages/wrangler/lib/functions/filepath-routing.test.ts Outdated Show resolved Hide resolved
packages/wrangler/package.json Show resolved Hide resolved
packages/wrangler/lib/functions/filepath-routing.ts Outdated Show resolved Hide resolved
packages/wrangler/lib/functions/buildWorker.ts Outdated Show resolved Hide resolved
packages/wrangler/lib/functions/buildWorker.ts Outdated Show resolved Hide resolved
packages/wrangler/src/pages.tsx Outdated Show resolved Hide resolved
@GregBrimble GregBrimble force-pushed the pages-functions-compiler branch 4 times, most recently from 4c098e5 to 81094f5 Compare December 23, 2021 15:30
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

I left a bunch of comments but I think most of them become moot once all the commits have been merged.

The one thing that concerns me is that the proxy child process is no longer being killed when the parent process is killed, right?

Other than that, I think this looks good to me.

packages/pages-functions-compiler/build.ts Outdated Show resolved Hide resolved
packages/wrangler/src/pages.tsx Outdated Show resolved Hide resolved
packages/wrangler/src/pages.tsx Outdated Show resolved Hide resolved
packages/wrangler/src/pages.tsx Outdated Show resolved Hide resolved
packages/wrangler/src/pages.tsx Show resolved Hide resolved
packages/wrangler/src/pages.tsx Outdated Show resolved Hide resolved
packages/wrangler/lib/functions/buildWorker.ts Outdated Show resolved Hide resolved
packages/wrangler/src/pages.tsx Outdated Show resolved Hide resolved
packages/wrangler/package.json Outdated Show resolved Hide resolved
packages/wrangler/package.json Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pages Relating to Pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: --watch mapped to Miniflare buildWatchPaths
4 participants