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

Refactor runtime packages to conform to the Remix server runtime interface #2359

Merged
merged 17 commits into from
Mar 21, 2022
Merged

Refactor runtime packages to conform to the Remix server runtime interface #2359

merged 17 commits into from
Mar 21, 2022

Conversation

pcattori
Copy link
Contributor

@pcattori pcattori commented Mar 17, 2022 •
edited
Loading

Currently, we use "magic" imports to expose the entire Remix API through the remix package.

import { anything } from 'remix'

To achieve this, we rely on the remix setup command to modify node_modules/remix post-installation. This is non-standard for an npm package and causes problems with tools like pnpm and Yarn Plug'n'Play.

Instead, we'd like to refactor to only use standard JS package conventions via four PRs:

  1. Refactoring for packages/* (and also including remix-deno) to provide a common API for server runtimes (node, cloudflare, deno) and a common API for adapters (express, netlify, cloudflare-workers, etc...). No breaking changes (this PR)
  2. Updates to templates, docs, examples mainly replacing remix "magic" imports with server runtime imports. No breaking changes
  3. Deprecation of remix "magic" imports (e.g. console.warn when people use remix imports). No breaking changes
  4. In a separate release: removal of the all deprecated behaviors from the previous three PRs. 🚨 Breaking changes 🚨

@pcattori pcattori force-pushed the pedro/refactor-runtime-packages branch from 8bff58c to cad4543 Compare March 17, 2022 17:30
mjackson
mjackson approved these changes Mar 17, 2022
View reviewed changes
Copy link
Member

@mjackson mjackson left a comment

Choose a reason for hiding this comment

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

This looks solid so far 👍

packages/remix-server-runtime/interface/cookies.ts Outdated Show resolved Hide resolved
packages/remix-server-runtime/interface/responses.ts Outdated Show resolved Hide resolved
packages/remix-server-runtime/interface/README.md Outdated Show resolved Hide resolved
@pcattori pcattori force-pushed the pedro/refactor-runtime-packages branch 10 times, most recently from 3872cc7 to d61a333 Compare March 18, 2022 13:52
@pcattori pcattori changed the title refactor(@remix-run/server-runtime): define RemixServerRuntime interface refactor(@remix-run/server-runtime): define Remix server runtime interface Mar 18, 2022
@pcattori pcattori changed the title refactor(@remix-run/server-runtime): define Remix server runtime interface Refactor runtime packages to conform to the Remix server runtime interface Mar 18, 2022
@pcattori pcattori force-pushed the pedro/refactor-runtime-packages branch 3 times, most recently from bad50d4 to 01756cc Compare March 18, 2022 19:45
mjackson
mjackson requested changes Mar 18, 2022
View reviewed changes
Copy link
Member

@mjackson mjackson left a comment

Choose a reason for hiding this comment

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

Very excited about this!

packages/remix-architect/__tests__/server-test.ts Show resolved Hide resolved
packages/remix-cloudflare/globals.ts Outdated Show resolved Hide resolved
packages/remix-server-runtime/README.md Outdated Show resolved Hide resolved
packages/remix-server-runtime/server.ts Outdated Show resolved Hide resolved
packages/remix-server-runtime/reexport.ts Outdated Show resolved Hide resolved
packages/remix-cloudflare/globals.ts Outdated Show resolved Hide resolved
packages/remix-cloudflare-workers/index.ts Show resolved Hide resolved
packages/remix-cloudflare-pages/index.ts Show resolved Hide resolved
@pcattori pcattori force-pushed the pedro/refactor-runtime-packages branch 5 times, most recently from 3dabd9c to be83e02 Compare March 19, 2022 03:46
mjackson
mjackson reviewed Mar 19, 2022
View reviewed changes
Copy link
Member

@mjackson mjackson left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looking good. I think the only thing that remains to be done is the examples, which we can do in an other PR so this one doesn't get too big.

Would you agree?

Edit: I totally spaced it on docs changes. I think those should be done in this PR as well.

packages/remix-cloudflare/index.ts Outdated Show resolved Hide resolved
packages/remix-cloudflare/sessions/KVSessionStorage.ts Outdated Show resolved Hide resolved
packages/remix-server-runtime/README.md Outdated Show resolved Hide resolved
@pcattori pcattori force-pushed the pedro/refactor-runtime-packages branch 2 times, most recently from fab8c58 to b77aae0 Compare March 20, 2022 14:29
@pcattori pcattori force-pushed the pedro/refactor-runtime-packages branch from bc4e760 to bd2fadf Compare March 21, 2022 18:24
@pcattori pcattori merged commit 8cd7f65 into dev Mar 21, 2022
@pcattori pcattori deleted the pedro/refactor-runtime-packages branch March 21, 2022 18:30
@pcattori pcattori mentioned this pull request Mar 21, 2022
Merged
MichaelDeBoey pushed a commit to TheRealAstoo/remix that referenced this pull request Mar 21, 2022
@pcattori @MichaelDeBoey
Refactor runtime packages to conform to the Remix server runtime inte…
957a85c 
…rface (remix-run#2359)

* feat(@remix-run/server-runtime): define interface for remix server runtimes

* refactor(@remix-run/server-runtime): organize exports

group types that should be re-exported by each remix server runtime

* refactor(@remix-run/server-runtime): factories for crypto-dependent interface functions

* docs(@remix-run/server-runtime): rewrite readme to explain the "remix server runtime interface"

* refactor(@remix-run/node): conform to the remix server runtime interface

instead of relying on magic exports

* refactor(@remix-run/node): implement crypto-dependent interface functions

* feat(@remix-run/cloudflare): create new server runtime package for cloudflare

* refactor(@remix-run/cloudflare): implement crypto-dependent interface functions

* refactor(remix-deno): conform to the remix server runtime interface

* refactor(remix-deno): implement crypto-dependent interface functions

* refactor(@remix-run/architect): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/express): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/netlify): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/vercel): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/cloudflare-workers): use `@remix-run/cloudflare` instead of `@remix-run/server-runtime`

* refactor(@remix-run/cloudflare-pages): use `@remix-run/cloudflare` instead of `@remix-run/server-runtime`

* refactor(server-runtimes,adapters): remove unused `platform` arg from `createRequestHandler`
mcansh pushed a commit that referenced this pull request Mar 21, 2022
@pcattori @mcansh
Refactor runtime packages to conform to the Remix server runtime inte…
7736444 
…rface (#2359)

* feat(@remix-run/server-runtime): define interface for remix server runtimes

* refactor(@remix-run/server-runtime): organize exports

group types that should be re-exported by each remix server runtime

* refactor(@remix-run/server-runtime): factories for crypto-dependent interface functions

* docs(@remix-run/server-runtime): rewrite readme to explain the "remix server runtime interface"

* refactor(@remix-run/node): conform to the remix server runtime interface

instead of relying on magic exports

* refactor(@remix-run/node): implement crypto-dependent interface functions

* feat(@remix-run/cloudflare): create new server runtime package for cloudflare

* refactor(@remix-run/cloudflare): implement crypto-dependent interface functions

* refactor(remix-deno): conform to the remix server runtime interface

* refactor(remix-deno): implement crypto-dependent interface functions

* refactor(@remix-run/architect): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/express): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/netlify): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/vercel): use `@remix-run/node` instead of `@remix-run/server-runtime`

* refactor(@remix-run/cloudflare-workers): use `@remix-run/cloudflare` instead of `@remix-run/server-runtime`

* refactor(@remix-run/cloudflare-pages): use `@remix-run/cloudflare` instead of `@remix-run/server-runtime`

* refactor(server-runtimes,adapters): remove unused `platform` arg from `createRequestHandler`
@mjackson mjackson mentioned this pull request Mar 21, 2022
Merged
@3x071c 3x071c mentioned this pull request Mar 24, 2022
Closed
@pcattori pcattori mentioned this pull request Mar 27, 2022
Closed
This was referenced Mar 29, 2022
Closed
Closed
Closed
Merged
@penx penx mentioned this pull request Apr 1, 2022
Closed
silvenon added a commit to silvenon/silvenon.com that referenced this pull request Apr 6, 2022
@silvenon
Remove magic Remix imports
e674620 
remix-run/remix#2359
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelDeBoey MichaelDeBoey MichaelDeBoey left review comments

@mjackson mjackson mjackson approved these changes

Assignees
No one assigned
Labels
CLA Signed package:dev package:server-runtime runtime:cloudflare runtime:node template:deno
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pcattori @mjackson @MichaelDeBoey