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

Use @vercel/nft to create lambdas #4969

Merged
merged 14 commits into from
May 24, 2022
Merged

Use @vercel/nft to create lambdas #4969

merged 14 commits into from
May 24, 2022

Conversation

Rich-Harris
Copy link
Member

An alternative to #4967fixes #4933.

This is something I've been meaning to look into for a while. By using @vercel/nft, we can get a list of all the files a function depends on, and copy them into the function directory rather than relying on a bundler like esbuild to bundle everything into a single file.

Doing so has a couple of advantages:

I think continuing to use esbuild for Edge Functions is the right strategy, at least for the time being (since these can't use native dependencies anyway).

There's some trickery involved in getting this to work — in order for relative imports to continue working, we need to find the common ancestor directory of all dependencies (which could even be outside the workspace, if using linked dependencies) and copy the structure into the lambda, and to support pnpm we need special handling for symlinks. It's working, though it feels like quite a lot of grunt work, so I'm not 100% sure I'm doing it right.

Todo:

  • use the same process for the v1 API

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented May 17, 2022

🦋 Changeset detected

Latest commit: 228cf07

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/adapter-vercel 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

@benmccann benmccann added the pkg:adapter-vercel Pertaining to the Vercel adapter label May 17, 2022
@Rich-Harris
Copy link
Member Author

Getting it to work with the v1 API is proving finicky, but since v3 is coming out of beta soon I think we should just revert to CJS output for v1 and count down the days until we can remove it altogether.

People who want to use top-level await will need to opt into v3 by setting the ENABLE_VC_BUILD=1 environment variable until it leaves beta.

Future optimisation: would be good to deduplicate shared dependencies between functions. Not going to worry about that right now though.

@Rich-Harris Rich-Harris marked this pull request as ready for review May 17, 2022 18:19
@Rich-Harris
Copy link
Member Author

Builds are completing successfully with this PR, but the build process on Vercel is then hanging with messages like these:

Attempting to remove file /vercel/path0/node_modules/.cache/turbo/c5e740db3d8bebe3/sites/kit.svelte.dev/.vercel/output/functions/render.func/node_modules/.pnpm/[email protected]/node_modules/onigasm; a subdirectory is required

Attempting to remove file /vercel/path0/node_modules/.cache/turbo/c5e740db3d8bebe3/sites/kit.svelte.dev/.vercel/output/functions/render.func/node_modules/.pnpm/[email protected]/node_modules/yallist; a subdirectory is required

@Rich-Harris
Copy link
Member Author

This should be fixed in the next version of turbo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked by upstream pkg:adapter-vercel Pertaining to the Vercel adapter
Projects
None yet
3 participants