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: implemented Remix server adapter and runtime for Netlify #16

Merged
merged 43 commits into from
Feb 8, 2023
Merged

feat: implemented Remix server adapter and runtime for Netlify #16

merged 43 commits into from
Feb 8, 2023

Conversation

nickytonline
Copy link
Contributor

@nickytonline nickytonline commented Jan 25, 2023 •
edited
Loading

Description

Related Tickets & Documents

Disregard the deploy failed check. I'm having issues unlinking it from the repository. It's not related to the project.

QA Instructions, Screenshots, Recordings

Local Development Steps

  1. Checkout this branch, e.g. via the GitHub CLI, gh co 16
  2. Run npm install
  3. Ensure you have the Netlify CLI installed/updated. See https://docs.netlify.com/cli/get-started/
  4. Run ntl build --offline
  5. Run ntl serve
  6. The local version of the edge demo site loads.

CleanShot 2023-02-08 at 07 43 46

Testing Deployed Site

  1. Ensure the deploy preview, one of the checks below, loads and that if you look in the Network panel of the devtools in the browser, that it has the cache control header set, i.e. cache-control: public, max-age=31536000, s-maxage=31536000

CleanShot 2023-02-07 at 16 53 02

For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

A hamster eating spaghetti

@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jan 25, 2023
@nickytonline nickytonline force-pushed the nickytonline/add-adapter-and-server-runtime branch from 417c5e2 to 7809450 Compare January 25, 2023 22:31
@nickytonline nickytonline self-assigned this Jan 25, 2023
@nickytonline nickytonline force-pushed the nickytonline/add-adapter-and-server-runtime branch from 6929ef9 to e141d98 Compare January 26, 2023 02:39
nickytonline
nickytonline commented Jan 26, 2023
View reviewed changes
LICENSE Show resolved Hide resolved
LICENSE Show resolved Hide resolved
@nickytonline @ascorbic
chore: remove extension from import
379fcf9 
Co-authored-by: Matt Kane <m@mk.gg>
@ascorbic
Copy link
Contributor

I wonder what we should do about Netlify Functions. I know this is edge-only now, but presumably we'll need to move the Functions adapter in here too. I wonder if we need to plan for this in naming, structure etc

@nickytonline nickytonline force-pushed the nickytonline/add-adapter-and-server-runtime branch from 1158976 to a58e06e Compare January 27, 2023 15:46
@nickytonline
Copy link
Contributor Author

I wonder what we should do about Netlify Functions. I know this is edge-only now, but presumably we'll need to move the Functions adapter in here too. I wonder if we need to plan for this in naming, structure etc

I've taken this into account which is why the repo is remix-compute and not remix-compute-edge. And I do plan on adding the Netlify Functions post first publish of this.

We could rename @netlify/remix-server-runtime to @netlify-remix-server-edge-runtime and the Netlify Functions one could be @netlify/remix-server-runtime.

Definitely open to suggestions here.

@nickytonline
Copy link
Contributor Author

One other thing @ascorbic. I think it makes sense to have the template live here instead of in our existing repo https://github.com/netlify/remix-edge-template since this will contain both the Netlify Functions and Netlify Edge adapter/runtimes.

Thoughts?

@ascorbic
Copy link
Contributor

It would make sense, but Deploy To Netlify doesn't support monorepos, so sadly not possible

@ascorbic
Copy link
Contributor

There's no Netlify Functions server runtime: it just uses the Node runtime. We could have @netlify/remix-edge-adapter and @netlify/remix-functions-adapter. Maybe this is one to ask others internally

nickytonline
nickytonline commented Feb 8, 2023
View reviewed changes
packages/remix-edge-adapter/src/server.ts
@@ -38,6 +38,10 @@ export function createRequestHandler({
const loadContext = (await getLoadContext?.(request, context)) || context

const response = await remixHandler(request, loadContext)

// A useful header for debugging
response.headers.set('x-nf-runtime', 'Edge')
Copy link
Contributor Author

@nickytonline nickytonline Feb 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

As discussed in #16 (comment) @ascorbic, we now have the x-nf-runtime header.

CleanShot 2023-02-08 at 09 04 15

@nickytonline
Copy link
Contributor Author

The label-pr actions are hanging for some reason. It's not blocking anything, they've just been slow to respond the past two days on this repo for some reason.

stephmarie17
stephmarie17 reviewed Feb 8, 2023
View reviewed changes
packages/remix-runtime/README.md Outdated
# Remix Server Runtime for Netlify

The Remix Server Runtime for Netlify is a serverless edge runtime for [Remix](https://remix.run) apps. It's built on top
of [Netlify Edge Functions](https://docs.netlify.com/functions/overview/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
of [Netlify Edge Functions](https://docs.netlify.com/functions/overview/).
of [Netlify Edge Functions](https://docs.netlify.com/edge-functions/overview/).

Here we can link to the Edge Functions page of the docs instead of the Functions page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I thought I updated that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think like I mention below, there's an issue with latest commits not appearing on GitHub or something, because this was the README before your comments. 🤔

# Remix Server Runtime for Netlify

The Remix Server Runtime for Netlify is a serverless edge runtime for [Remix](https://remix.run) apps. It's built on top
of [Netlify Edge Functions](https://docs.netlify.com/edge-functions/overview/).

stephmarie17
stephmarie17 reviewed Feb 8, 2023
View reviewed changes
Copy link
Contributor

@stephmarie17 stephmarie17 left a comment

Choose a reason for hiding this comment

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

Left a few minor suggestions to add some clarity and replace README URLs but otherwise READMEs good!

stephmarie17
stephmarie17 approved these changes Feb 8, 2023
View reviewed changes
Copy link
Contributor

@stephmarie17 stephmarie17 left a comment

Choose a reason for hiding this comment

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

READMEs look good 🚀

@nickytonline @stephmarie17
Update packages/edge-demo-site/README.md
6ed4e65 
Co-authored-by: Stephanie <52582720+stephmarie17@users.noreply.github.com>
@nickytonline nickytonline enabled auto-merge (squash) February 8, 2023 23:38
@nickytonline nickytonline enabled auto-merge (squash) February 8, 2023 23:39
@nickytonline nickytonline enabled auto-merge (squash) February 8, 2023 23:40
@nickytonline nickytonline merged commit 9efc5cf into main Feb 8, 2023
@nickytonline nickytonline deleted the nickytonline/add-adapter-and-server-runtime branch February 8, 2023 23:41
This was referenced Feb 9, 2023
Closed
Closed
Closed
Closed
Closed
Merged
This was referenced Feb 13, 2023
Closed
Merged
@nickytonline nickytonline mentioned this pull request Feb 14, 2023
Merged
5 tasks
@nickytonline nickytonline mentioned this pull request Feb 21, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephmarie17 stephmarie17 stephmarie17 approved these changes

@ascorbic ascorbic ascorbic approved these changes

Assignees

@nickytonline nickytonline

Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement Remix server adapter and server runtime for Netlify
4 participants
@nickytonline @ascorbic @stephmarie17 @MarcL