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.

Sign up for GitHub

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

Jump to bottom

feat(qwik-city): add vercel serverless adapter #6355

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

feat(qwik-city): add vercel serverless adapter #6355

wants to merge 24 commits into from
+2,101 −278

Conversation

okikio
Copy link
Contributor

@okikio okikio commented May 22, 2024 •
edited
Loading

Overview

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Description

This PR introduces a new Vercel Serverless Adapter for the Qwik framework. The adapter allows developers to deploy Qwik applications as serverless functions on Vercel, leveraging the serverless architecture for scalable and efficient performance. This addition provides a seamless way to deploy Qwik applications to Vercel's platform, expanding the deployment options available to developers.

Use cases and why

    1. Deploy Qwik applications as serverless functions on Vercel for improved scalability and performance.
    1. Simplify the deployment process to Vercel by providing a dedicated adapter that integrates smoothly with the Qwik framework.
    1. Enable developers to leverage Vercel's serverless functions for faster content delivery and optimized user experiences.

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • [~] I have made corresponding changes to the documentation
  • [~] Added new tests to cover the fix / functionality

@netlify Netlify
Copy link

netlify bot commented May 22, 2024 •
edited
Loading

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 6e49c79

@PatrickJS PatrickJS self-assigned this May 22, 2024
@PatrickJS PatrickJS mentioned this pull request May 22, 2024
Closed
@PatrickJS
Copy link
Member

we need to also run pnpm api-update

@PatrickJS PatrickJS marked this pull request as ready for review May 22, 2024 22:04
@okikio okikio requested review from a team as code owners May 25, 2024 23:27
@okikio okikio requested a review from PatrickJS May 25, 2024 23:29
@PatrickJS PatrickJS requested a review from a team as a code owner June 20, 2024 00:28
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Jun 20, 2024 •
edited
Loading

Open in Stackblitz

npm i https://pkg.pr.new/@builder.io/qwik@6355

npm i https://pkg.pr.new/@builder.io/qwik-city@6355

npm i https://pkg.pr.new/eslint-plugin-qwik@6355

npm i https://pkg.pr.new/create-qwik@6355

commit: bbebd75

PatrickJS
PatrickJS previously approved these changes Jun 20, 2024
View reviewed changes
wmertens
wmertens requested changes Jul 30, 2024
View reviewed changes
Copy link
Member

@wmertens wmertens left a comment

Choose a reason for hiding this comment

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

LGTM, but please change the import naming scheme. Also merge main or rebase, and add the typescript re-exports under packages/qwik-city/adapters (see the examples in there)

packages/qwik-city/package.json Outdated Show resolved Hide resolved
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Aug 1, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: bbebd75

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

This PR includes changesets to release 4 packages
Name Type
@builder.io/qwik-city Patch
eslint-plugin-qwik Patch
@builder.io/qwik Patch
create-qwik 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

@okikio
Copy link
Contributor Author

okikio commented Aug 1, 2024

The pr has now become too big imo, but I don't know if splitting it into smaller prs is a good idea, as that might leave things in a semi-broken state.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 5, 2024 •
edited
Loading

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview bbebd75

wmertens
wmertens requested changes Oct 6, 2024
View reviewed changes
Copy link
Member

@wmertens wmertens left a comment

Choose a reason for hiding this comment

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

LGTM, I don't think this PR is too big, it's a single change but it touches a lot of files. So just some more TS changes required to make sure there's no breakage between versions

packages/qwik/src/optimizer/src/qwik-binding-map.ts Outdated
Copy link
Member

Choose a reason for hiding this comment

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

please undo this

packages/docs/src/routes/api/qwik/api.json Outdated
Copy link
Member

Choose a reason for hiding this comment

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

please run pnpm api.update

packages/qwik-city/adapters/vercel-edge/vite.d.ts Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep this so TS doesn't suddenly complain

packages/qwik-city/middleware/vercel/edge.d.ts Outdated
Copy link
Member

Choose a reason for hiding this comment

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

same here, let's keep this one around

packages/qwik-city/package.json
"types": "./lib/adapters/vercel-edge/vite/index.d.ts",
"import": "./lib/adapters/vercel-edge/vite/index.mjs",
"require": "./lib/adapters/vercel-edge/vite/index.cjs"
"types": "./lib/adapters/vercel/edge/vite/index.d.ts",
Copy link
Member

Choose a reason for hiding this comment

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

actually, let's give this different types that re-export /edge/vite/index but add /** @deprecated Please import from adapters/vercel/edge instead */ to all of them

packages/qwik-city/package.json
@@ -140,8 +150,17 @@
"require": "./lib/middleware/request-handler/index.cjs"
},
"./middleware/vercel-edge": {
"types": "./lib/middleware/vercel-edge/index.d.ts",
"import": "./lib/middleware/vercel-edge/index.mjs"
"types": "./lib/middleware/vercel/edge/index.d.ts",
Copy link
Member

Choose a reason for hiding this comment

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

same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wmertens wmertens wmertens requested changes

@mhevery mhevery Awaiting requested review from mhevery

@shairez shairez Awaiting requested review from shairez

@PatrickJS PatrickJS Awaiting requested review from PatrickJS

Requested changes must be addressed to merge this pull request.

Assignees

@PatrickJS PatrickJS

Labels
None yet
Projects
Qwik Development
Status: Backlog
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@okikio @PatrickJS @wmertens