Skip to content

docs(decisions): fix grammar + typos #10660

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ Additionally, NPM packages can be accessed as Deno modules via [Deno-friendly CD

Remix has some requirements around dependencies:

- Remix treeshakes dependencies that are free of side-effects.
- Remix tree shakes dependencies that are free of side effects.
- Remix sets the environment (dev/prod/test) across all code, including dependencies, at runtime via the `NODE_ENV` environment variable.
- Remix depends on some NPM packages that should be specified as peer dependencies (notably, `react` and `react-dom`).

### Treeshaking
### Tree shaking

To optimize bundle size, Remix [treeshakes](https://esbuild.github.io/api/#tree-shaking) your app's code and dependencies.
To optimize bundle size, Remix [tree shakes](https://esbuild.github.io/api/#tree-shaking) your app's code and dependencies.
This also helps to separate browser code and server code.

Under the hood, the Remix compiler uses [esbuild](https://esbuild.github.io).
Under the hood, the Remix compiler uses [`esbuild`](https://esbuild.github.io).
Like other bundlers, `esbuild` uses [`sideEffects` in `package.json` to determine when it is safe to eliminate unused imports](https://esbuild.github.io/api/#conditionally-injecting-a-file).

Unfortunately, URL imports do not have a standard mechanism for marking packages as side-effect free.
Unfortunately, URL imports do not have a standard mechanism for marking packages as side-effect-free.

### Setting dev/prod/test environment

Expand All @@ -37,7 +37,7 @@ That means changing environment requires changing the URL import in the source c
While you could use multiple import maps (`dev.json`, `prod.json`, etc...) to workaround this, import maps have other limitations:

- standard tooling for managing import maps is not available
- import maps are not composeable, so any dependencies that use import maps must be manually accounted for
- import maps are not composable, so any dependencies that use import maps must be manually accounted for

### Specifying peer dependencies

Expand All @@ -46,10 +46,9 @@ That means that specifying peer dependencies becomes tedious and error-prone as

- determine which dependencies themselves depend on `react` (or other similar peer dependency), even if indirectly.
- manually figure out which `react` version works across _all_ of these dependencies
- set that version for `react` as a query parameter in _all_ of the URLs for the identified dependencies
- set that version for `react` as a query parameter in _all_ the URLs for the identified dependencies

If any dependencies change (added, removed, version change),
the user must repeat all of these steps again.
If any dependencies change (added, removed, version change), the user must repeat all of these steps.

## Decision

Expand All @@ -59,7 +58,7 @@ Do not use Deno-friendly CDNs for NPM dependencies in Remix projects using Deno.

Use `npm` and `node_modules/` to manage NPM dependencies like `react` for Remix projects, even when using Deno with Remix.

Deno module dependencies (e.g. from `https://deno.land`) can still be managed via URL imports.
Deno module dependencies (e.g., from `https://deno.land`) can still be managed via URL imports.

### Allow URL imports

Expand All @@ -68,7 +67,7 @@ letting your browser runtime and server runtime handle them accordingly.
That means that you may:

- use URL imports for the browser
- use URL imports for the server, if your server runtime supports it
- use URL imports for the server if your server runtime supports it

For example, Node will throw errors for URL imports, while Deno will resolve URL imports as normal.

Expand All @@ -78,7 +77,7 @@ Remix will not yet support import maps.

## Consequences

- URL imports will not be treeshaken.
- URL imports will not be tree shaken.
- Users can specify environment via the `NODE_ENV` environment variable at runtime.
- Users won't have to do error-prone, manual dependency resolution.

Expand Down
6 changes: 3 additions & 3 deletions decisions/0002-do-not-clone-request.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Do not clone request
# Do not clone a request

Date: 2022-05-13

Expand All @@ -10,10 +10,10 @@ To allow multiple loaders / actions to read the body of a request, we have been

## Decision

Do not clone requests before they are passed to user-code (actions, handleDocumentRequest, handleDataRequest), and remove body from request passed to loaders. Loaders should be thought of as a "GET" / "HEAD" request handler. These request methods are not allowed to have a body, therefore you should not be reading it in your Remix loader function.
Do not clone requests before they are passed to user-code (actions, handleDocumentRequest, handleDataRequest), and remove the body from the `request` passed to `loader`s. Loaders should be thought of as a "GET" / "HEAD" request handler. These request methods are not allowed to have a body; therefore, you should not be reading it in your Remix loader function.

## Consequences

Loaders always receive a null body for the request.

If you are reading the request body in both an action and handleDocumentRequest or handleDataRequest this will now fail as the body will have already been read. If you wish to continue reading the request body in multiple places for a single request against recommendations, consider using `.clone()` before reading it; just know this comes with tradeoffs.
If you are reading the request body in both an action and `handleDocumentRequest` or `handleDataRequest` this will now fail as the body will have already been read. If you wish to continue reading the request body in multiple places for a single request against recommendations, consider using `.clone()` before reading it; know this comes with tradeoffs.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default function Route() {
}
```

For end-to-end type safety, it is then the user's responsability to make sure that `loader` and `action` also use the same type in the `json` generic:
For end-to-end type safety, it is then the user's responsibility to make sure that `loader` and `action` also use the same type in the `json` generic:

```ts
export const loader: LoaderFunction = () => {
Expand Down Expand Up @@ -85,7 +85,7 @@ function useLoaderData<T>(): T {
}
```

`useLoaderData` isn't basing its return type on how `data` was set (i.e. the return value of `loader`) nor is it validating the data.
`useLoaderData` isn't basing its return type on how `data` was set (i.e., the return value of `loader`) nor is it validating the data.
It's just blindly casting `data` to whatever the user passed in for the generic `T`.

### Issues with current approach
Expand Down Expand Up @@ -119,15 +119,15 @@ Again, the same goes for `useActionData`.
- Return type of `useLoaderData` and `useActionData` should somehow be inferred from `loader` and `action`, not blindly type cast
- Return type of `loader` and `action` should be inferred
- Necessarily, return type of `json` should be inferred from its input
- No module side-effects (so higher-order functions like `makeLoader` is definitely a no).
- No module side effects (so higher-order functions like `makeLoader` is definitely a no).
- `json` should allow everything that `JSON.stringify` allows.
- `json` should allow only what `JSON.stringify` allows.
- `useLoaderData` should not return anything that `JSON.parse` can't return.

### Key insight: `loader` and `action` are an _implicit_ inputs
### Key insight: `loader` and `action` are _implicit_ inputs

While there's been interest in inferring the types for `useLoaderData` based on `loader`, there was [hesitance to use a Typescript generic to do so](https://github.com/remix-run/remix/pull/3276#issuecomment-1164764821).
Typescript generics are apt for specifying or inferring types for _inputs_, not for blindly type casting output types.
While there's been interest in inferring the types for `useLoaderData` based on `loader`, there was [hesitance to use a TypeScript generic to do so](https://github.com/remix-run/remix/pull/3276#issuecomment-1164764821).
TypeScript generics are apt for specifying or inferring types for _inputs_, not for blindly type casting output types.

A key factor in the decision was identifying that `loader` and `action` are _implicit_ inputs of `useLoaderData` and `useActionData`.

Expand All @@ -146,15 +146,15 @@ Without the `loader` argument to infer types from, `useLoaderData` needs a way t
Additionally, `loader` and `useLoaderData` are both managed by Remix across the network.
While its true that Remix doesn't "own" the network in the strictest sense, having `useLoaderData` return data that does not correspond to its `loader` is an exceedingly rare edge-case.

Same goes for `useActionData`.
The same goes for `useActionData`.

---

A similar case is how [Prisma](https://www.prisma.io/) infers types from database schemas available at runtime, even though there are (exceedingly rare) edge-cases where that database schema _could_ be mutated after compile-time but before run-time.

## Decision

Explicitly provide type of the implicit `loader` input for `useLoaderData` and then infer the return type for `useLoaderData`.
Explicitly provide a type of the implicit `loader` input for `useLoaderData` and then infer the return type for `useLoaderData`.
Do the same for `action` and `useActionData`.

```ts
Expand Down Expand Up @@ -209,13 +209,13 @@ export default function Route() {
## Consequences

- Users can continue to provide non-inferred types by type casting the result of `useLoaderData` or `useActionData`
- Users can opt-in to inferred types by using `typeof loader` or `typeof action` at the generic for `useLoaderData` or `useActionData`.
- Users can opt in to inferred types by using `typeof loader` or `typeof action` at the generic for `useLoaderData` or `useActionData`.
- Return types for `loader` and `action` will be the sources-of-truth for the types inferred for `useLoaderData` and `useActionData`.
- Users do not need to write redundant code to align types across the network
- Return type of `useLoaderData` and `useActionData` will correspond to the JSON _serialized_ types from `json` calls in `loader` and `action`, eliminating a class of errors.
- `LoaderFunction` and `ActionFunction` should not be used when opting into type inference as they override the inferred return types.[^1]

🚨 Users who opt-in to inferred types **MUST** return a `TypedResponse` from `json` and **MUST NOT** return a bare object:
🚨 Users who opt in to inferred types **MUST** return a `TypedResponse` from `json` and **MUST NOT** return a bare object:

```ts
const loader = () => {
Expand All @@ -227,4 +227,4 @@ const loader = () => {
};
```

[^1]: The proposed `satisfies` operator for Typescript would let `LoaderFunction` and `ActionFunction` enforce function types while preserving the narrower inferred return type: https://github.com/microsoft/TypeScript/issues/47920
[^1]: The proposed `satisfies` operator for TypeScript would let `LoaderFunction` and `ActionFunction` enforce function types while preserving the narrower inferred return type: https://github.com/microsoft/TypeScript/issues/47920
20 changes: 10 additions & 10 deletions decisions/0004-streaming-apis.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ Status: accepted

Remix aims to provide first-class support for React 18's streaming capabilities. Throughout the development process we went through many iterations and naming schemes around the APIs we plan to build into Remix to support streaming, so this document aims to lay out the final names we chose and the reasons behind it.

It's also worth nothing that even in a single-page-application without SSR-streaming, the same concepts still apply so these decisions were made with React Router 6.4.0 in mind as well - which will support the same Data APIs from Remix.
It's also worth nothing that even in a single-page-application without SSR streaming, the same concepts still apply, so these decisions were made with React Router 6.4.0 in mind as well which will support the same Data APIs from Remix.

## Decision

Streaming in Remix can be thought of as having 3 touch points with corresponding APIs:
Streaming in Remix can be thought of as having three touchpoints with corresponding APIs:

1. _Initiating_ a streamed response in your `loader` can be done by returning a `defer(object)` call from your `loader` in which some of the keys on `object` are `Promise` instances
2. _Accessing_ a streamed response from `useLoaderData`
Expand All @@ -32,7 +32,7 @@ In the spirit of `#useThePlatform` we've chosen to leverage the `Promise` API to

### Initiating

In order to initiate a streamed response in your `loader`, you can use the `defer()` utility which accepts a JSON object with `Promise` values from your `loader`.
To initiate a streamed response in your `loader`, you can use the `defer()` utility which accepts a JSON object with `Promise` values from your `loader`.

```tsx
export async function loader() {
Expand Down Expand Up @@ -83,8 +83,8 @@ function exampleLoader3() {
<summary>Other considered API names:</summary>
<br/>
<ul>
<li><code>deferred()</code> - This is just a bit of a weird word that doesn't have much pre-existing semantic meaning. Is this the <code>jQuery.Deferred</code> thing from back in the day? Remix in general wants to avoid needlessly introducing net-new language to an already convoluted landscape!</li>
<li><code>stream()</code> - We also thought <code>stream</code> might be a good name since that's what the call is telling Remix to do - stream the responses down to the browser. But - this is also potentially misleading because stream is ambiguous in ths case. Developers may mistakenly think that this gives them back a <code>Stream</code> instance and they can arbitrarily send multiple chunks of data down to the browser over time. This is not how the current API works - but also seems like a really interesting idea for Remix to consider in the future, so we wanted to keep the <code>stream()</code> name available for future use cases.</li>
<li><code>deferred()</code> - This is just a bit of a unique word that has little pre-existing semantic meaning. Is this the <code>jQuery.Deferred</code> thing from back in the day? Remix in general wants to avoid needlessly introducing net-new language to an already convoluted landscape!</li>
<li><code>stream()</code> - We also thought <code>stream</code> might be a good name since that's what the call is telling Remix to do stream the responses down to the browser. But - this is also potentially misleading because stream is ambiguous in this case. Developers may mistakenly think that this gives them back a <code>Stream</code> instance and they can arbitrarily send multiple chunks of data down to the browser over time. This is not how the current API works but also seems like a really interesting idea for Remix to consider in the future, so we wanted to keep the <code>stream()</code> name available for future use cases.</li>
</ul>
</details>

Expand All @@ -102,9 +102,9 @@ function Component() {

### Rendering

In order to render your `Promise` values from `useLoaderData()`, Remix provides a new `<Await>` component which handles rendering the resolved value, or propagating the rejected value through an `errorElement` or further upwards to the Route-level error boundaries. In order to access the resolved or rejected values, there are two new hooks that only work in the context of an `<Await>` component - `useAsyncValue()` and `useAsyncError()`.
To render your `Promise` values from `useLoaderData()`, Remix provides a new `<Await>` component which handles rendering the resolved value, or propagating the rejected value through an `errorElement` or further upwards to the Route-level error boundaries. To access the resolved or rejected values, there are two new hooks that only work in the context of an `<Await>` component - `useAsyncValue()` and `useAsyncError()`.

This examples shows the full set of render-time APIs:
This example shows the full set of render-time APIs:

```tsx
function Component() {
Expand Down Expand Up @@ -132,7 +132,7 @@ function MyError() {

Note that `useAsyncValue` and `useAsyncError` only work in the context of an `<Await>` component.

The `<Await>` name comes from the fact that for these lazily-rendered promises, we're not `await`-ing the promise in our loader, so instead we need to `<Await>` the promise in our render function and provide a fallback UI. The `resolve` prop is intended to mimic how you'd await a resolved value in plain Javascript:
The `<Await>` name comes from the fact that for these lazily rendered promises, we're not `await`-ing the promise in our loader, so instead we need to `<Await>` the promise in our render function and provide a fallback UI. The `resolve` prop is intended to mimic how you'd await a resolved value in plain JavaScript:

```tsx
// This JSX:
Expand Down Expand Up @@ -181,12 +181,12 @@ If you do not provide an `errorElement`, then promise rejections will bubble up
<br>
<p>We originally implemented this as a <code>&lt;Deferred value={promise} fallback={&lt;Loader /&gt;} errorElement={&lt;MyError/&gt;} /></code>, but eventually we chose to remove the built-in <code>&lt;Suspense&gt;</code> boundary for better composability and eventual use with <code>&lt;SuspenseList&gt;</code>. Once that was removed, and we were only using a <code>Promise</code> it made sense to move to a generic <code>&lt;Await&gt;</code> component that could be used with <em>any</em> promise, not just those coming from <code>defer()</code> in a <code>loader</code></p>

<p>We also considered various alternatives for the hook names - most notably `useResolvedValue`/`useRejectedValue`. However, these were a bit too tightly coupled to the `Promise` nomenclature. Remember, `Await` supports non-Promise values as well as render-errors, so it would be confusing if `useResolvedValue` was handing you a non-Promise value, or if `useRejectedValue` was handing you a render error from a resolved `Promise`. `useAsyncValue`/`useAsyncError` better encompasses those scenarios as well.</p>
<p>We also considered various alternatives for the hook names most notably `useResolvedValue`/`useRejectedValue`. However, these were a bit too tightly coupled to the `Promise` nomenclature. Remember, `Await` supports non-Promise values as well as render-errors, so it would be confusing if `useResolvedValue` was handing you a non-Promise value, or if `useRejectedValue` was handing you a render error from a resolved `Promise`. `useAsyncValue`/`useAsyncError` better encompasses those scenarios as well.</p>
</details>

## React Router Notes

With the presence of the `<Await>` component in React Router and because the Promise's don't have to be serialized over the network - you can _technically_ just return raw Promise values on a naked object from your loader. However, this is strongly discouraged because the router will be unaware of these promises and thus won't be able to cancel them if the user navigates away prior to the promise settling.
With the presence of the `<Await>` component in React Router and because the Promise's don't have to be serialized over the network - you can _technically_ return raw Promise values on a naked object from your loader. However, this is strongly discouraged because the router will be unaware of these promises and thus won't be able to cancel them if the user navigates away before the promise settling.

By forcing users to call the `defer()` utility, we ensure that the router is able to track the in-flight promises and properly cancel them. It also allows us to handle synchronous rendering of promises that resolve prior to other critical data. Without the `defer()` utility these raw Promises would need to be thrown by the `<Await>` component to the `<Suspense>` boundary a single time to unwrap the value, resulting in a UI flicker.

Expand Down
Loading