-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat: add @remix-run/netlify-edge
package + update Netlify template
#3104
Changes from 122 commits
03b037f
299f041
a98927a
0558147
3beff72
803c643
d092036
e7fb4cb
74bb317
590c309
3289337
9d0bf37
664de49
581ff1c
3ba4e09
31d193c
efcb3c4
8467d2d
99a8184
3a3e6b1
4ea0dad
8ad0f6f
ffc921d
07ee7f6
b92e1ad
d97eac3
11d0180
52a1369
2fb574f
be0fe0b
efef1d4
25c0d7e
b529737
d87302e
91a524c
f81397f
df0b43a
994edc8
dcba4e9
099df65
0503e2e
c7b9802
926255d
fb6f12c
c3c1bff
e4c0d6a
3318f9f
1e3dcf6
806fc74
2df3cc6
86f1afd
0b542fe
989d025
10b2695
0d78477
803f9bf
6195e51
b37e849
a96c7b7
f69d22b
c9afffb
96e84b7
896a352
e2b59d6
6037bb6
138c10c
aca10a1
bb9e29f
48d76d5
0a353f9
0f64dbb
8cf637e
f81dbe0
3196d98
60fbe48
6f1a643
835a555
d87126e
b17d74a
b708933
31e16f2
caae8bd
4ddb5eb
7a39ad4
aed7635
eaf50ff
02eb35b
147a2dc
d44be97
7126e42
bf7c380
fe69ed8
b9a3029
ea2af16
7fbc19e
abf0ce9
8f73241
cf1f398
c8c9fb4
5db90ef
d96b4b3
49ea16e
d9c4634
a8dfa62
9ed5799
1166b49
525f528
d5332f8
a85cc3f
74d4c23
b384a5a
21f143d
d113f8a
1636c01
00cb7af
3ac5b9f
675ed9d
f1e431f
c896016
7cdfca7
36ae8d8
90a9235
c419ae8
8f1a6de
d0e737e
bc672cc
a8c1db1
212aecf
adb9639
0346753
733559f
29763da
cef75f1
b82df5c
e136beb
f96d44f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,9 @@ | |
], | ||
"imports": { | ||
"@remix-run/server-runtime": "https://esm.sh/@remix-run/server-runtime@nightly", | ||
"@remix-run/deno": "../packages/remix-deno/index.ts", | ||
"@remix-run/deno/": "../packages/remix-deno/", | ||
MichaelDeBoey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"@netlify/edge-functions": "https://edge.netlify.com", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should add these to the template's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, they're compiled away by then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since We also include it in the template once #3104 (comment) is fixed CC/ @pcattori There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Since Deno supports TS, I think we don't need to compile @pcattori can probably give more insights in what's best here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's imported by code that's then compiled by node and rollup. Using an import map won't make any difference, because rollup will ignore it and just load and bundle There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't compile Whatever we decide to do, I think we should streamline it in both the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just trying to see the benefit here. I have it working here, https://github.com/nickytonline/remix/pull/3. Even though our edge function offering uses Deno, the projects people create, at least at the moment are always Node.js projects. We donβt have a deno.json and our tooling is Node.js centric, i,e, the Netlify CLI. As well, they may add Netlify functions for certain things, e,g, scheduled functions, background functions or even a custom function, and these are Node.js based. |
||
"mime": "https://esm.sh/mime@3.0.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# Welcome to Remix! | ||
|
||
[Remix](https://remix.run) is a web framework that helps you build better websites with React. | ||
|
||
To get started, open a new shell and run: | ||
|
||
```sh | ||
npx create-remix@latest | ||
``` | ||
|
||
Then follow the prompts you see in your terminal. | ||
|
||
For more information about Remix, [visit remix.run](https://remix.run)! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import "@remix-run/deno/globals.ts"; | ||
|
||
// @ts-ignore | ||
globalThis.process ||= { env: Deno.env.toObject() }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import "./globals.ts"; | ||
|
||
export type { GetLoadContextFunction, RequestHandler } from "./server.ts"; | ||
export { createRequestHandler } from "./server.ts"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
{ | ||
"name": "@remix-run/netlify-edge", | ||
"description": "Netlify Edge server request handler for Remix", | ||
"version": "1.8.0", | ||
"license": "MIT", | ||
"main": "dist/index.js", | ||
"types": "dist/index.d.ts", | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/remix-run/remix", | ||
"directory": "packages/remix-netlify-edge" | ||
}, | ||
"bugs": { | ||
"url": "https://github.com/remix-run/remix/issues" | ||
}, | ||
"dependencies": { | ||
"@remix-run/deno": "1.8.0" | ||
}, | ||
"devDependencies": { | ||
"@netlify/edge-functions": "^2.0.0" | ||
nickytonline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
"peerDependencies": { | ||
"@netlify/edge-functions": "^2.0.0" | ||
}, | ||
"engines": { | ||
"node": ">=14" | ||
}, | ||
"files": [ | ||
"dist/", | ||
"CHANGELOG.md", | ||
"LICENSE.md", | ||
"README.md" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
const { getAdapterConfig } = require("../../rollup.utils"); | ||
|
||
/** @returns {import("rollup").RollupOptions[]} */ | ||
module.exports = function rollup() { | ||
return [getAdapterConfig("netlify-edge")]; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
import { createRemixRequestHandler } from "@remix-run/deno"; | ||
import type { AppLoadContext, ServerBuild } from "@remix-run/deno"; | ||
import type { Context } from "@netlify/edge-functions"; | ||
|
||
type LoadContext = AppLoadContext & Context; | ||
|
||
/** | ||
* A function that returns the value to use as `context` in route `loader` and | ||
* `action` functions. | ||
* | ||
* You can think of this as an escape hatch that allows you to pass | ||
* environment/platform-specific values through to your loader/action. | ||
*/ | ||
export type GetLoadContextFunction = ( | ||
request: Request, | ||
context: Context | ||
) => Promise<LoadContext> | LoadContext; | ||
|
||
export type RequestHandler = ( | ||
request: Request, | ||
context: LoadContext | ||
) => Promise<Response | void>; | ||
|
||
export function createRequestHandler({ | ||
build, | ||
mode, | ||
getLoadContext, | ||
}: { | ||
build: ServerBuild; | ||
mode?: string; | ||
getLoadContext?: GetLoadContextFunction; | ||
}): RequestHandler { | ||
let remixHandler = createRemixRequestHandler(build, mode); | ||
|
||
let assetPath = build.assets.url.split("/").slice(0, -1).join("/"); | ||
|
||
return async ( | ||
request: Request, | ||
context: LoadContext | ||
): Promise<Response | void> => { | ||
let { pathname } = new URL(request.url); | ||
// Skip the handler for static files | ||
if (pathname.startsWith(`${assetPath}/`)) { | ||
return; | ||
} | ||
try { | ||
let loadContext = (await getLoadContext?.(request, context)) || context; | ||
|
||
let response = await remixHandler(request, loadContext); | ||
if (response.status === 404) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just include a separate |
||
// Check if there is a matching static file | ||
let originResponse = await context.next({ | ||
sendConditionalRequest: true, | ||
}); | ||
if (originResponse.status !== 404) { | ||
return originResponse; | ||
} | ||
} | ||
return response; | ||
} catch (error: unknown) { | ||
console.error(error); | ||
|
||
return new Response("Internal Error", { status: 500 }); | ||
} | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{ | ||
"include": ["**/*.ts"], | ||
"compilerOptions": { | ||
"lib": ["ES2019", "WebWorker"], | ||
"target": "ES2019", | ||
"moduleResolution": "node", | ||
"allowSyntheticDefaultImports": true, | ||
"strict": true, | ||
"declaration": true, | ||
"emitDeclarationOnly": true, | ||
"outDir": "../../build/node_modules/@remix-run/netlify-edge", | ||
"rootDir": ".", | ||
// Avoid naming conflicts between history and react-router-dom relying on | ||
// lib.dom.d.ts Window and this being a WebWorker env. | ||
"skipLibCheck": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,5 +2,7 @@ node_modules | |
|
||
/.cache | ||
/public/build | ||
/.netlify | ||
.env | ||
|
||
# Local Netlify folder | ||
.netlify |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
# Welcome to Remix! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This template should be a repo somewhere on the Netlify GH org. This |
||
|
||
- [Remix Docs](https://remix.run/docs) | ||
- [Netlify Functions](https://www.netlify.com/products/functions/) | ||
- [Netlify Functions Overview](https://docs.netlify.com/functions/overview) | ||
|
||
## Netlify Setup | ||
|
||
|
@@ -34,7 +34,7 @@ netlify init | |
The Remix dev server starts your app in development mode, rebuilding assets on file changes. To start the Remix dev server: | ||
|
||
```sh | ||
npm run dev | ||
netlify dev | ||
``` | ||
|
||
Open up [http://localhost:3000](http://localhost:3000), and you should be ready to go! | ||
|
@@ -51,7 +51,7 @@ Note: When running the Netlify CLI, file changes will rebuild assets, but you wi | |
|
||
## Deployment | ||
|
||
There are two ways to deploy your app to Netlify, you can either link your app to your git repo and have it auto deploy changes to Netlify, or you can deploy your app manually. If you've followed the setup instructions already, all you need to do is run this: | ||
There are two ways to deploy your app to Netlify, you can either link your app to your git repository and have it auto deploy changes to Netlify, or you can deploy your app manually. If you've followed the setup instructions already, all you need to do is run this: | ||
|
||
```sh | ||
# preview deployment | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,14 @@ | ||
[build] | ||
command = "remix build" | ||
publish = "public" | ||
|
||
[dev] | ||
command = "remix watch" | ||
port = 3000 | ||
command = "remix build" | ||
publish = "public" | ||
|
||
[[redirects]] | ||
from = "/*" | ||
to = "/.netlify/functions/server" | ||
status = 200 | ||
from = "/*" | ||
to = "/.netlify/functions/server" | ||
status = 200 | ||
|
||
[[headers]] | ||
for = "/build/*" | ||
[headers.values] | ||
"Cache-Control" = "public, max-age=31536000, s-maxage=31536000" | ||
for = "/build/*" | ||
|
||
[headers.values] | ||
"Cache-Control" = "public, max-age=31536000, s-maxage=31536000" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,12 @@ | ||
/** @type {import('@remix-run/dev').AppConfig} */ | ||
module.exports = { | ||
serverBuildTarget: "netlify", | ||
server: | ||
process.env.NETLIFY || process.env.NETLIFY_LOCAL | ||
? "./server.js" | ||
: undefined, | ||
ignoredRouteFiles: ["**/.*"], | ||
serverBuildPath: ".netlify/functions-internal/server.js", | ||
// appDirectory: "app", | ||
// assetsBuildDirectory: "public/build", | ||
// serverBuildPath: ".netlify/functions-internal/server.js", | ||
// publicPath: "/build/", | ||
}; |
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.
Maybe we should also add it here for consistency?
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.
We don't want it here because it's literally only for CI for Netlify Edge. Just the presence of the environment variable is all we care about which is why I didn't include it for the Netlify Functions deploy.
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.
Our integration tests are also rewritten to be explicit about both cases, so that's why I think we should do it here as well
remix/integration/form-test.ts
Line 468 in 8f7100b
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.
env vars are strings, so you'd need to change the comparison logic if you do this, because
"false"
is truthy.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.
It wouldn't be a problem to do the following check imo