- No contacts -
+ No contacts +
Name
@@ -915,41 +1021,35 @@ The route name `_index` is special. It tells Remix to match and render this rout -Voila! No more blank space. It's common to put dashboards, stats, feeds, etc. at index routes. They can participate in data loading as well. +Voilร ! No more blank space. It's common to put dashboards, stats, feeds, etc. at index routes. They can participate in data loading as well. ## Cancel Button On the edit page we've got a cancel button that doesn't do anything yet. We'd like it to do the same thing as the browser's back button. -We'll need a click handler on the button as well as [`useNavigate`][usenavigate]. +We'll need a click handler on the button as well as [`useNavigate`][use-navigate]. ๐ **Add the cancel button click handler with `useNavigate`** -```jsx filename=app/routes/contacts.$contactId.edit.tsx lines=[1,7,17-19] +```tsx filename=app/routes/contacts.$contactId_.edit.tsx lines=[5,11,18] +// existing imports import { Form, useLoaderData, - useNavigation, + useNavigate, } from "@remix-run/react"; +// existing imports & exports -/* existing code*/ - -export default function Edit() { - const contact = useLoaderData(); +export default function EditContact() { + const { contact } = useLoaderData(); const navigate = useNavigate(); return ( - - {/* existing code */} - + + {/* existing elements */} Save - { - navigate(-1); - }} - > + navigate(-1)} type="button"> Cancel @@ -966,41 +1066,47 @@ A ``, while seemingly redundant, is the HTML way of preven Two more features to go. We're on the home stretch! -## URL Search Params and GET Submissions +## `URLSearchParams` and `GET` Submissions -All of our interactive UI so far have been either links that change the URL or forms that post data to actions. The search field is interesting because it's a mix of both: it's a form but it only changes the URL, it doesn't change data. +All of our interactive UI so far have been either links that change the URL or `form`s that post data to `action` functions. The search field is interesting because it's a mix of both: it's a `form`, but it only changes the URL, it doesn't change data. Let's see what happens when we submit the search form: ๐ **Type a name into the search field and hit the enter key** -Note the browser's URL now contains your query in the URL as [URLSearchParams][urlsearchparams]: +Note the browser's URL now contains your query in the URL as [`URLSearchParams`][url-search-params]: ``` http://localhost:3000/?q=ryan ``` -Since it's not ``, Remix emulates the browser by serializing the [`FormData`][formdata] into the [`URLSearchParams`][urlsearchparams] instead of the request body. +Since it's not ``, Remix emulates the browser by serializing the [`FormData`][form-data] into the [`URLSearchParams`][url-search-params] instead of the request body. + +`loader` functions have access to the search params from the `request`. Let's use it to filter the list: -Loaders have access to the search params from the `request`. Let's use it to filter the list: +๐ **Filter the list if there are `URLSearchParams`** -๐ **Filter the list if there are URLSearchParams** +```tsx filename=app/root.tsx lines=[3,9-11] +import type { + LinksFunction, + LoaderArgs, +} from "@remix-run/node"; -```tsx filename=app/routes/root.tsx -/* existing imports */ -import { type LoaderArgs, redirect } from "@remix-run/node"; +// existing imports & exports -export async function loader({ request }: LoaderArgs) { +export const loader = async ({ request }: LoaderArgs) => { const url = new URL(request.url); const q = url.searchParams.get("q"); const contacts = await getContacts(q); - return { contacts }; -} + return json({ contacts }); +}; + +// existing code ``` -Because this is a GET, not a POST, Remix _does not_ call the `action`. Submitting a GET form is the same as clicking a link: only the URL changes. +Because this is a `GET`, not a `POST`, Remix _does not_ call the `action` function. Submitting a `GET` `form` is the same as clicking a link: only the URL changes. This also means it's a normal page navigation. You can click the back button to get back to where you were. @@ -1015,44 +1121,47 @@ In other words, the URL and our input's state are out of sync. Let's solve (2) first and start the input with the value from the URL. -๐ **Return `q` from your loader, set it as the input's default value** +๐ **Return `q` from your `loader`, set it as the input's default value** -```tsx filename=app/routes/root.tsx lines=[7,11,26] -// existing code +```tsx filename=app/root.tsx lines=[7,11,23] +// existing imports & exports -export async function loader({ request }: LoaderArg) { +export const loader = async ({ request }: LoaderArgs) => { const url = new URL(request.url); const q = url.searchParams.get("q"); const contacts = await getContacts(q); - return { contacts, q }; -} + return json({ contacts, q }); +}; export default function Root() { - const { contacts, q } = useLoaderData(); + const { contacts, q } = useLoaderData(); const navigation = useNavigation(); return ( - <> - - Remix Contacts - - - - {/* existing code */} - - {/* existing code */} + + {/* existing elements */} + + + {/* existing elements */} + + + + {/* existing elements */} + + {/* existing elements */} + + {/* existing elements */} - {/* existing code */} - - {/* existing code */} - + {/* existing elements */} + + ); } ``` @@ -1061,16 +1170,17 @@ The input field will show the query if you refresh the page after a search now. Now for problem (1), clicking the back button and updating the input. We can bring in `useEffect` from React to manipulate the input's value in the DOM directly. -๐ **Synchronize input value with the URL Search Params** +๐ **Synchronize input value with the `URLSearchParams`** -```tsx filename=app/routes/root.tsx lines=[1,9-14] +```tsx filename=app/root.tsx lines=[2,10-15] +// existing imports import { useEffect } from "react"; -// existing code +// existing imports & exports export default function Root() { + const { contacts, q } = useLoaderData(); const navigation = useNavigation(); - const { contacts, q } = useLoaderData(); useEffect(() => { const searchField = document.getElementById("q"); @@ -1085,54 +1195,60 @@ export default function Root() { > ๐ค Shouldn't you use a controlled component and React State for this? -You could certainly do this as a controlled component. You will have more synchronization points but it's up to you. +You could certainly do this as a controlled component. You will have more synchronization points, but it's up to you. + Expand this to see what it would look like -```jsx filename=app/routes/root.tsx lines=[9,13-15,30,32-34] +```tsx filename=app/root.tsx lines=[2,9-10,12-16,30-33,36-37] +// existing imports import { useEffect, useState } from "react"; -// existing code + +// existing imports & exports export default function Root() { + const { contacts, q } = useLoaderData(); const navigation = useNavigation(); - const { contacts, q } = useLoaderData(); - // the query now needs to be kept in state - const [query, setQuery] = useState(q); + const [query, setQuery] = useState(q || ""); // we still have a `useEffect` to synchronize the query // to the component state on back/forward button clicks useEffect(() => { - setQuery(q); + setQuery(q || ""); }, [q]); return ( - <> - - Remix Contacts - - - { - setQuery(e.target.value); - }} - /> - {/* existing code */} - - {/* existing code */} + + {/* existing elements */} + + + {/* existing elements */} + + + + setQuery(event.currentTarget.value) + } + placeholder="Search" + type="search" + // switched to `value` from `defaultValue` + value={query} + /> + {/* existing elements */} + + {/* existing elements */} + + {/* existing elements */} - {/* existing code */} - - + {/* existing elements */} + + ); } ``` @@ -1141,56 +1257,66 @@ export default function Root() { Alright, you should now be able to click the back/forward/refresh buttons and the input's value should be in sync with the URL and results. -## Submitting Forms `onChange` +## Submitting `Form`'s `onChange` -We've got a product decision to make here. Sometimes you want the user to submit the form to filter some results, other times you want to filter as the user types. We've already implemented the first, so let's see what it's like for the second. +We've got a product decision to make here. Sometimes you want the user to submit the `form` to filter some results, other times you want to filter as the user types. We've already implemented the first, so let's see what it's like for the second. -We've seen `useNavigate` already, we'll use its cousin, [`useSubmit`][usesubmit], for this. +We've seen `useNavigate` already, we'll use its cousin, [`useSubmit`][use-submit], for this. -```tsx filename=src/routes/root.tsx lines=[4,10,25-27] -// existing code +```tsx filename=app/root.tsx lines=[13,20,33-35] +// existing imports import { - // existing code + Form, + Links, + LiveReload, + Meta, + NavLink, + Outlet, + Scripts, + ScrollRestoration, + useLoaderData, + useNavigation, useSubmit, } from "@remix-run/react"; +// existing imports & exports export default function Root() { + const { contacts, q } = useLoaderData(); const navigation = useNavigation(); - const { contacts, q } = useLoaderData(); const submit = useSubmit(); + // existing code + return ( - <> - - Remix Contacts - - - { - submit(event.currentTarget.form); - }} - /> - {/* existing code */} - - {/* existing code */} + + {/* existing elements */} + + + {/* existing elements */} + + + submit(event.currentTarget) + } + role="search" + > + {/* existing elements */} + + {/* existing elements */} + + {/* existing elements */} - {/* existing code */} - - {/* existing code */} - + {/* existing elements */} + + ); } ``` -As you type, the form is automatically submitted now! +As you type, the `form` is automatically submitted now! -Note the argument to [`submit`][usesubmit]. The `submit` function will serialize and submit any form you pass to it. We're passing in `event.currentTarget.form`. The `currentTarget` is the DOM node the event is attached to (the input), and the `currentTarget.form` is the form the input belongs to. +Note the argument to [`submit`][use-submit]. The `submit` function will serialize and submit any form you pass to it. We're passing in `event.currentTarget`. The `currentTarget` is the DOM node the event is attached to (the `form`). ## Adding Search Spinner @@ -1198,7 +1324,7 @@ In a production app, it's likely this search will be looking for records in a da Without any loading indicator, the search feels kinda sluggish. Even if we could make our database faster, we'll always have the user's network latency in the way and out of our control. -For a better user experience, let's add some immediate UI feedback for the search. We'll use [`useNavigation`][usenavigation] again. +For a better user experience, let's add some immediate UI feedback for the search. We'll use [`useNavigation`][use-navigation] again. ๐ **Add a variable to know if we're searching** @@ -1222,7 +1348,7 @@ export default function Root() { When nothing is happening, `navigation.location` will be `undefined`, but when the user navigates it will be populated with the next location while data loads. Then we check if they're searching with `location.search`. -๐ **Add classes to search form elements using the state** +๐ **Add classes to search form elements using the new `searching` state** ```tsx filename=app/routes/root.tsx lines=[3,14] @@ -1248,17 +1374,32 @@ When nothing is happening, `navigation.location` will be `undefined`, but when t Bonus points, avoid fading out the main screen when searching: -```tsx filename=app/routes/root.tsx - - - +```tsx filename=app/root.tsx lines=[13] +// existing imports & exports + +export default function Root() { + // existing code + + return ( + + {/* existing elements */} + + {/* existing elements */} + + + + {/* existing elements */} + + + ); +} ``` You should now have a nice spinner on the left side of the search input. @@ -1267,7 +1408,7 @@ You should now have a nice spinner on the left side of the search input. ## Managing the History Stack -Since the form is submitted for every key stroke, typing the characters "alex" and then deleting them with backspace results in a huge history stack ๐. We definitely don't want this: +Since the form is submitted for every keystroke, typing the characters "alex" and then deleting them with backspace results in a huge history stack ๐. We definitely don't want this: @@ -1275,135 +1416,162 @@ We can avoid this by _replacing_ the current entry in the history stack with the ๐ **Use `replace` in `submit`** -```tsx filename=app/routes/root.tsx lines=[6-9] - - { - const isFirstSearch = q == null; - submit(event.currentTarget.form, { - replace: !isFirstSearch, - }); - }} - /> - {/* existing code */} - +```tsx filename=app/root.tsx lines=[16-19] +// existing imports & exports + +export default function Root() { + // existing code + + return ( + + {/* existing elements */} + + + {/* existing elements */} + + { + const isFirstSearch = q === null; + submit(event.currentTarget, { + replace: !isFirstSearch, + }); + }} + role="search" + > + {/* existing elements */} + + {/* existing elements */} + + {/* existing elements */} + + {/* existing elements */} + + + ); +} ``` After a quick check if this is the first search or not, we decide to replace. Now the first search will add a new entry, but every keystroke after that will replace the current entry. Instead of clicking back 7 times to remove the search, users only have to click back once. -## Forms Without Navigation +## `Form`s Without Navigation -So far all of our forms have changed the URL. While these user flows are common, it's equally as common to want to submit a form _without_ causing a navigation. +So far all of our forms have changed the URL. While these user flows are common, it's equally common to want to submit a form _without_ causing a navigation. -For these cases, we have [`useFetcher`][usefetcher]. It allows us to communicate with loaders and actions without causing a navigation. +For these cases, we have [`useFetcher`][use-fetcher]. It allows us to communicate with `action`s and `loader`s without causing a navigation. -The โ button on the contact page makes sense for this. We aren't creating or deleting a new record and we don't want to change pages. We simply want to change the data on the page we're looking at. +The โ button on the contact page makes sense for this. We aren't creating or deleting a new record, and we don't want to change pages. We simply want to change the data on the page we're looking at. ๐ **Change the `` form to a fetcher form** -```tsx filename=app/routes/contacts.$contactId.edit.tsx lines=[1,6,10,20] +```tsx filename=app/routes/contacts.$contactId.tsx lines=[4,14,18,30] +// existing imports import { - useLoaderData, Form, useFetcher, + useLoaderData, } from "@remix-run/react"; +// existing imports & exports // existing code -function Favorite({ contact }: { contact: ContactRecord }) { +const Favorite: FunctionComponent<{ + contact: Pick; +}> = ({ contact }) => { const fetcher = useFetcher(); const favorite = contact.favorite; return ( {favorite ? "โ " : "โ"} ); -} +}; ``` -This form will no longer cause a navigation, but simply fetch to the our action. Speaking of which ... this won't work until we create the action. +This form will no longer cause a navigation, but simply fetch to the `action`. Speaking of which ... this won't work until we create the `action`. -๐ **Create the action** +๐ **Create the `action`** -```tsx filename=app/routes/contacts.$contactId.tsx -// existing code +```tsx filename=app/routes/contacts.$contactId.tsx lines=[2,7,10-19] import type { ActionArgs, LoaderArgs, } from "@remix-run/node"; +// existing imports -import { getContact, updateContact } from "../data"; +import type { ContactRecord } from "../data"; +// existing imports -export async function action({ - request, +export const action = async ({ params, -}: ActionArgs) { + request, +}: ActionArgs) => { invariant(params.contactId, "Missing contactId param"); const formData = await request.formData(); return updateContact(params.contactId, { favorite: formData.get("favorite") === "true", }); -} +}; + +// existing code ``` Alright, we're ready to click the star next to the user's name! -Check that out, both stars automatically update. Our new `` works almost exactly like a the `` we've been using: it calls the action and then all data is revalidated automatically--even your errors will be caught the same way. +Check that out, both stars automatically update. Our new `` works almost exactly like the `` we've been using: it calls the action and then all data is revalidated automatically โ even your errors will be caught the same way. There is one key difference though, it's not a navigation, so the URL doesn't change and the history stack is unaffected. ## Optimistic UI -You probably noticed the app felt kind of unresponsive when we clicked the the favorite button from the last section. Once again, we added some network latency because you're going to have it in the real world. +You probably noticed the app felt kind of unresponsive when we clicked the favorite button from the last section. Once again, we added some network latency because you're going to have it in the real world. -To give the user some feedback, we could put the star into a loading state with [`fetcher.state`][fetcherstate] (a lot like `navigation.state` from before), but we can do something even better this time. We can use a strategy called "Optimistic UI" +To give the user some feedback, we could put the star into a loading state with [`fetcher.state`][fetcher-state] (a lot like `navigation.state` from before), but we can do something even better this time. We can use a strategy called "Optimistic UI" -The fetcher knows the form data being submitted to the action, so it's available to you on `fetcher.formData`. We'll use that to immediately update the star's state, even though the network hasn't finished. If the update eventually fails, the UI will revert to the real data. +The fetcher knows the [`FormData`][form-data] being submitted to the `action`, so it's available to you on `fetcher.formData`. We'll use that to immediately update the star's state, even though the network hasn't finished. If the update eventually fails, the UI will revert to the real data. ๐ **Read the optimistic value from `fetcher.formData`** ```tsx filename=app/routes/contacts.$contactId.tsx lines=[7-9] // existing code -function Favorite({ contact }: { contact: ContactRecord }) { +const Favorite: FunctionComponent<{ + contact: Pick; +}> = ({ contact }) => { const fetcher = useFetcher(); - - let favorite = contact.favorite; - if (fetcher.formData) { - favorite = fetcher.formData.get("favorite") === "true"; - } + const favorite = fetcher.formData + ? fetcher.formData.get("favorite") === "true" + : contact.favorite; return ( {favorite ? "โ " : "โ"} ); -} +}; ``` Now the star _immediately_ changes to the new state when you click it. @@ -1412,44 +1580,30 @@ Now the star _immediately_ changes to the new state when you click it. That's it! Thanks for giving Remix a shot. We hope this tutorial gives you a solid start to build great user experiences. There's a lot more you can do, so make sure to check out all the APIs ๐ -[vite]: https://vitejs.dev/guide/ -[node]: https://nodejs.org -[createbrowserrouter]: ../routers/create-browser-router -[route]: ../route/route -[tutorial-css]: https://gist.githubusercontent.com/ryanflorence/ba20d473ef59e1965543fa013ae4163f/raw/499707f25a5690d490c7b3d54c65c65eb895930c/react-router-6.4-tutorial-css.css -[tutorial-data]: https://gist.githubusercontent.com/ryanflorence/1e7f5d3344c0db4a8394292c157cd305/raw/f7ff21e9ae7ffd55bfaaaf320e09c6a08a8a6611/contacts.js -[routeelement]: ../route/route#element -[jim]: https://blog.jim-nielsen.com/ -[errorelement]: ../route/error-element -[userouteerror]: ../hooks/use-route-error -[isrouteerrorresponse]: ../utils/is-route-error-response +[jim]: https://blog.jim-nielsen.com [outlet]: ../components/outlet [link]: ../components/link -[setup]: #setup [loader]: ../route/loader -[useloaderdata]: ../hooks/use-loader-data +[use-loader-data]: ../hooks/use-loader-data [action]: ../route/action [params]: ../route/loader#params [form]: ../components/form [request]: https://developer.mozilla.org/en-US/docs/Web/API/Request -[formdata]: https://developer.mozilla.org/en-US/docs/Web/API/FormData -[fromentries]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/fromEntries -[requestformdata]: https://developer.mozilla.org/en-US/docs/Web/API/Request/formData +[form-data]: https://developer.mozilla.org/en-US/docs/Web/API/FormData +[object-from-entries]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/fromEntries +[request-form-data]: https://developer.mozilla.org/en-US/docs/Web/API/Request/formData [response]: https://developer.mozilla.org/en-US/docs/Web/API/Response -[redirect]: ../fetch/redirect -[returningresponses]: ../route/loader#returning-responses -[usenavigation]: ../hooks/use-navigation -[index]: ../route/route#index -[path]: ../route/route#path -[usenavigate]: ../hooks/use-navigate -[uselocation]: ../hooks/use-location -[urlsearchparams]: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams -[usesubmit]: ../hooks/use-submit -[navlink]: ../components/nav-link -[usefetcher]: ../hooks/use-fetcher -[fetcherstate]: ../hooks/use-fetcher#fetcherstate -[assetbuilddir]: ../file-conventions/remix-config#assetsbuilddirectory +[redirect]: ../utils/redirect +[returning-response-instances]: ../route/loader#returning-response-instances +[use-navigation]: ../hooks/use-navigation +[use-navigate]: ../hooks/use-navigate +[url-search-params]: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams +[use-submit]: ../hooks/use-submit +[nav-link]: ../components/nav-link +[use-fetcher]: ../hooks/use-fetcher +[fetcher-state]: ../hooks/use-fetcher#fetcherstate +[assets-build-directory]: ../file-conventions/remix-config#assetsbuilddirectory [links]: ../route/links -[notfound]: ../guides/not-found -[routeconvention]: ../file-conventions/route-files-v2 +[routes-file-conventions]: ../file-conventions/routes [quickstart]: ./quickstart +[fetch]: https://developer.mozilla.org/en-US/docs/Web/API/fetch diff --git a/integration/fetcher-test.ts b/integration/fetcher-test.ts index 216e83d3421..9f33e7411d0 100644 --- a/integration/fetcher-test.ts +++ b/integration/fetcher-test.ts @@ -245,8 +245,11 @@ test.describe("useFetcher", () => { method: "get", }), ]); - - await page.waitForSelector(`pre:has-text("${LUNCH}")`); + // Check full HTML here - Chromium/Firefox/Webkit seem to render this in + // a but Edge puts it in some weird code editor markup: + // + // "LUNCH" + expect(await app.getHtml()).toContain(LUNCH); }); test("Form can hit an action", async ({ page }) => { @@ -259,7 +262,11 @@ test.describe("useFetcher", () => { method: "post", }), ]); - await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`); + // Check full HTML here - Chromium/Firefox/Webkit seem to render this in + // a but Edge puts it in some weird code editor markup: + // + // "LUNCH" + expect(await app.getHtml()).toContain(CHEESESTEAK); }); }); diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 743dd98b152..c513017ff4c 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -137,7 +137,7 @@ export async function createAppFixture(fixture: Fixture, mode?: ServerMode) { let newChunk = chunk.toString(); stdout += newChunk; let match: RegExpMatchArray | null = stdout.match( - /started at http:\/\/localhost:(\d+)\s/ + /\[remix-serve\] http:\/\/localhost:(\d+)\s/ ); if (match) { clearTimeout(rejectTimeout); @@ -223,6 +223,26 @@ export async function createFixtureProject( path.join(projectDir, "node_modules"), { overwrite: true } ); + // let remixDev = path.join( + // projectDir, + // "node_modules/@remix-run/dev/dist/cli.js" + // ); + // await fse.chmod(remixDev, 0o755); + // await fse.ensureSymlink( + // remixDev, + // path.join(projectDir, "node_modules/.bin/remix") + // ); + // + // let remixServe = path.join( + // projectDir, + // "node_modules/@remix-run/serve/dist/cli.js" + // ); + // await fse.chmod(remixServe, 0o755); + // await fse.ensureSymlink( + // remixServe, + // path.join(projectDir, "node_modules/.bin/remix-serve") + // ); + await writeTestFiles(init, projectDir); // We update the config file *after* writing test files so that tests can provide a custom diff --git a/integration/helpers/node-template/package.json b/integration/helpers/node-template/package.json index e202cff6ca6..05953d0ec46 100644 --- a/integration/helpers/node-template/package.json +++ b/integration/helpers/node-template/package.json @@ -6,7 +6,7 @@ "scripts": { "build": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js build", "dev": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js dev", - "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js build" + "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js ./build/index.js" }, "dependencies": { "@remix-run/node": "0.0.0-local-version", diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts deleted file mode 100644 index 750b893430d..00000000000 --- a/integration/hmr-log-test.ts +++ /dev/null @@ -1,561 +0,0 @@ -import { test, expect } from "@playwright/test"; -import execa from "execa"; -import fs from "node:fs"; -import path from "node:path"; -import type { Readable } from "node:stream"; -import getPort, { makeRange } from "get-port"; - -import type { FixtureInit } from "./helpers/create-fixture.js"; -import { - createFixtureProject, - css, - js, - json, -} from "./helpers/create-fixture.js"; - -test.setTimeout(120_000); - -let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ - config: { - dev: { - port: options.devPort, - }, - }, - files: { - "package.json": json({ - private: true, - sideEffects: false, - type: "module", - scripts: { - dev: `node ./node_modules/@remix-run/dev/dist/cli.js dev -c "node ./server.js"`, - }, - dependencies: { - "@remix-run/css-bundle": "0.0.0-local-version", - "@remix-run/node": "0.0.0-local-version", - "@remix-run/react": "0.0.0-local-version", - "cross-env": "0.0.0-local-version", - express: "0.0.0-local-version", - isbot: "0.0.0-local-version", - react: "0.0.0-local-version", - "react-dom": "0.0.0-local-version", - tailwindcss: "0.0.0-local-version", - }, - devDependencies: { - "@remix-run/dev": "0.0.0-local-version", - "@types/react": "0.0.0-local-version", - "@types/react-dom": "0.0.0-local-version", - typescript: "0.0.0-local-version", - }, - engines: { - node: ">=18.0.0", - }, - }), - - "server.js": js` - import path from "path"; - import url from "url"; - import express from "express"; - import { createRequestHandler } from "@remix-run/express"; - import { logDevReady, installGlobals } from "@remix-run/node"; - - installGlobals(); - - const app = express(); - app.use(express.static("public", { immutable: true, maxAge: "1y" })); - - const MODE = process.env.NODE_ENV; - const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); - - app.all( - "*", - createRequestHandler({ - build: await import(BUILD_PATH), - mode: MODE, - }) - ); - - let port = ${options.appPort}; - app.listen(port, async () => { - let build = await import(BUILD_PATH); - console.log('โ app ready: http://localhost:' + port); - if (process.env.NODE_ENV === 'development') { - logDevReady(build); - } - }); - `, - - "tailwind.config.js": js` - /** @type {import('tailwindcss').Config} */ - module.exports = { - content: ["./app/**/*.{ts,tsx,jsx,js}"], - theme: { - extend: {}, - }, - plugins: [], - }; - `, - - "app/tailwind.css": css` - @tailwind base; - @tailwind components; - @tailwind utilities; - `, - - "app/styles.module.css": css` - .test { - color: initial; - } - `, - - "app/root.tsx": js` - import type { LinksFunction } from "@remix-run/node"; - import { Link, Links, LiveReload, Meta, Outlet, Scripts } from "@remix-run/react"; - import { cssBundleHref } from "@remix-run/css-bundle"; - - import Counter from "./components/counter"; - import styles from "./tailwind.css"; - - export const links: LinksFunction = () => [ - { rel: "stylesheet", href: styles }, - ...cssBundleHref ? [{ rel: "stylesheet", href: cssBundleHref }] : [], - ]; - - // dummy loader to make sure that HDR is granular - export const loader = () => { - return null; - }; - - export default function Root() { - return ( - - - - - - - - Root Input - - - - - Home - About - MDX - - - - - - - - - ); - } - `, - - "app/routes/_index.tsx": js` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - const t = useLoaderData(); - return ( - - Index Title - - ) - } - `, - - "app/routes/about.tsx": js` - import Counter from "../components/counter"; - export default function About() { - return ( - - About Title - - - ) - } - `, - "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' -export const loader = () => "crazy" -export const Component = () => { - const data = useLoaderData() - return {data} -} - -# heyo -whatsup - - -`, - - "app/components/counter.tsx": js` - import * as React from "react"; - export default function Counter({ id }) { - let [count, setCount] = React.useState(0); - return ( - - setCount(count + 1)}>inc {count} - - ); - } - `, - }, -}); - -let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - -let wait = async ( - callback: () => boolean, - { timeoutMs = 1000, intervalMs = 250 } = {} -) => { - let start = Date.now(); - while (Date.now() - start <= timeoutMs) { - if (callback()) { - return; - } - await sleep(intervalMs); - } - throw Error(`wait: timeout ${timeoutMs}ms`); -}; - -let bufferize = (stream: Readable): (() => string) => { - let buffer = ""; - stream.on("data", (data) => (buffer += data.toString())); - return () => buffer; -}; - -let logConsoleError = (error: Error) => { - console.error(`[console] ${error.name}: ${error.message}`); -}; - -let expectConsoleError = ( - isExpected: (error: Error) => boolean, - unexpected = logConsoleError -) => { - return (error: Error) => { - if (isExpected(error)) { - return; - } - unexpected(error); - }; -}; - -let HMR_TIMEOUT_MS = 10_000; - -test("HMR", async ({ page, browserName }) => { - // uncomment for debugging - // page.on("console", (msg) => console.log(msg.text())); - page.on("pageerror", logConsoleError); - let dataRequests = 0; - page.on("request", (request) => { - let url = new URL(request.url()); - if (url.searchParams.has("_data")) { - dataRequests++; - } - }); - - let portRange = makeRange(4080, 4099); - let appPort = await getPort({ port: portRange }); - let devPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject(fixture({ appPort, devPort })); - - // spin up dev server - let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); - let devStdout = bufferize(dev.stdout!); - let devStderr = bufferize(dev.stderr!); - try { - await wait( - () => { - if (dev.exitCode) throw Error("Dev server exited early"); - return /โ app ready: /.test(devStdout()); - }, - { timeoutMs: HMR_TIMEOUT_MS } - ); - - await page.goto(`http://localhost:${appPort}`, { - waitUntil: "networkidle", - }); - - // `` value as page state that - // would be wiped out by a full page refresh - // but should be persisted by hmr - let input = page.getByLabel("Root Input"); - expect(input).toBeVisible(); - await input.type("asdfasdf"); - - let counter = await page.waitForSelector("#root-counter"); - await counter.click(); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - let indexPath = path.join(projectDir, "app", "routes", "_index.tsx"); - let originalIndex = fs.readFileSync(indexPath, "utf8"); - let counterPath = path.join(projectDir, "app", "components", "counter.tsx"); - let originalCounter = fs.readFileSync(counterPath, "utf8"); - let cssModulePath = path.join(projectDir, "app", "styles.module.css"); - let originalCssModule = fs.readFileSync(cssModulePath, "utf8"); - let mdxPath = path.join(projectDir, "app", "routes", "mdx.mdx"); - let originalMdx = fs.readFileSync(mdxPath, "utf8"); - - // make content and style changed to index route - let newCssModule = ` - .test { - background: black; - color: white; - } - `; - fs.writeFileSync(cssModulePath, newCssModule); - - let newIndex = ` - import { useLoaderData } from "@remix-run/react"; - import styles from "~/styles.module.css"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - const t = useLoaderData(); - return ( - - Changed - - ) - } - `; - fs.writeFileSync(indexPath, newIndex); - - // detect HMR'd content and style changes - await page.waitForLoadState("networkidle"); - - let h1 = page.getByText("Changed"); - await h1.waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(h1).toHaveCSS("color", "rgb(255, 255, 255)"); - expect(h1).toHaveCSS("background-color", "rgb(0, 0, 0)"); - - // verify that `` value was persisted (i.e. hmr, not full page refresh) - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // undo change - fs.writeFileSync(indexPath, originalIndex); - fs.writeFileSync(cssModulePath, originalCssModule); - await page.getByText("Index Title").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // We should not have done any revalidation yet as only UI has changed - expect(dataRequests).toBe(0); - - // add loader - let withLoader1 = ` - import { json } from "@remix-run/node"; - import { useLoaderData } from "@remix-run/react"; - - export let loader = () => json({ hello: "world" }); - - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - let { hello } = useLoaderData(); - return ( - - Hello, {hello} - - ) - } - `; - fs.writeFileSync(indexPath, withLoader1); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(1); - await page.waitForLoadState("networkidle"); - - await page.getByText("Hello, world").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - expect(dataRequests).toBe(1); - - let withLoader2 = ` - import { json } from "@remix-run/node"; - import { useLoaderData } from "@remix-run/react"; - - export function loader() { - return json({ hello: "planet" }) - } - - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - let { hello } = useLoaderData(); - return ( - - Hello, {hello} - - ) - } - `; - fs.writeFileSync(indexPath, withLoader2); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(2); - await page.waitForLoadState("networkidle"); - - await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // change shared component - let updatedCounter = ` - import * as React from "react"; - export default function Counter({ id }) { - let [count, setCount] = React.useState(0); - return ( - - setCount(count - 1)}>dec {count} - - ); - } - `; - fs.writeFileSync(counterPath, updatedCounter); - await page.waitForSelector(`#root-counter:has-text("dec 1")`); - counter = await page.waitForSelector("#root-counter"); - await counter.click(); - await counter.click(); - await page.waitForSelector(`#root-counter:has-text("dec -1")`); - - await page.click(`a[href="/about"]`); - let aboutCounter = await page.waitForSelector( - `#about-counter:has-text("dec 0")` - ); - await aboutCounter.click(); - await page.waitForSelector(`#about-counter:has-text("dec -1")`); - - // undo change - fs.writeFileSync(counterPath, originalCounter); - - counter = await page.waitForSelector(`#root-counter:has-text("inc -1")`); - await counter.click(); - counter = await page.waitForSelector(`#root-counter:has-text("inc 0")`); - - aboutCounter = await page.waitForSelector( - `#about-counter:has-text("inc -1")` - ); - await aboutCounter.click(); - aboutCounter = await page.waitForSelector( - `#about-counter:has-text("inc 0")` - ); - - expect(dataRequests).toBe(2); - - // mdx - await page.click(`a[href="/mdx"]`); - await page.waitForSelector(`#crazy`); - let mdx = `import { useLoaderData } from '@remix-run/react' -export const loader = () => "hot" -export const Component = () => { - const data = useLoaderData() - return {data} -} - -# heyo -whatsup - - -`; - fs.writeFileSync(mdxPath, mdx); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(4); - await page.waitForSelector(`#hot`); - - fs.writeFileSync(mdxPath, originalMdx); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(5); - await page.waitForSelector(`#crazy`); - - // dev server doesn't crash when rebuild fails - await page.click(`a[href="/"]`); - await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); - await page.waitForLoadState("networkidle"); - - let stderr = devStderr(); - let withSyntaxError = ` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - eport efault functio Index() { - const t = useLoaderData(); - return ( - - With Syntax Error - - ) - } - `; - fs.writeFileSync(indexPath, withSyntaxError); - await wait( - () => - devStderr() - .replace(stderr, "") - .includes('Expected ";" but found "efault"'), - { - timeoutMs: HMR_TIMEOUT_MS, - } - ); - - // React Router integration w/ React Refresh has a bug where sometimes rerenders happen with old UI and new data - // in this case causing `TypeError: Cannot destructure property`. - // Need to fix that bug, but it only shows a harmless console error in the browser in dev - page.removeListener("pageerror", logConsoleError); - // let expectedErrorCount = 0; - let expectDestructureTypeError = expectConsoleError((error) => { - let expectedMessage = new Set([ - // chrome, edge - "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", - // firefox - "(intermediate value)() is null", - // webkit - "Right side of assignment cannot be destructured", - ]); - let isExpected = - error.name === "TypeError" && expectedMessage.has(error.message); - // if (isExpected) expectedErrorCount += 1; - return isExpected; - }); - page.on("pageerror", expectDestructureTypeError); - - let withFix = ` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - // const t = useLoaderData(); - return ( - - With Fix - - ) - } - `; - fs.writeFileSync(indexPath, withFix); - await page.waitForLoadState("networkidle"); - await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); - - // Restore normal console error handling - page.removeListener("pageerror", expectDestructureTypeError); - // expect(expectedErrorCount).toBe(browserName === "webkit" ? 1 : 2); - page.addListener("pageerror", logConsoleError); - } catch (e) { - console.log("stdout begin -----------------------"); - console.log(devStdout()); - console.log("stdout end -------------------------"); - - console.log("stderr begin -----------------------"); - console.log(devStderr()); - console.log("stderr end -------------------------"); - throw e; - } finally { - dev.kill(); - } -}); diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index 0339e97f5c1..dfec1d43ad8 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -1,9 +1,11 @@ -import { test, expect } from "@playwright/test"; -import execa from "execa"; import fs from "node:fs"; import path from "node:path"; import type { Readable } from "node:stream"; -import getPort, { makeRange } from "get-port"; +import type { Page } from "@playwright/test"; +import { test, expect } from "@playwright/test"; +import execa from "execa"; +import getPort from "get-port"; +import pidtree from "pidtree"; import type { FixtureInit } from "./helpers/create-fixture.js"; import { @@ -15,76 +17,8 @@ import { test.setTimeout(150_000); -let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ - config: { - dev: { - port: options.devPort, - }, - }, - files: { - "package.json": json({ - private: true, - sideEffects: false, - type: "module", - scripts: { - dev: `node ./node_modules/@remix-run/dev/dist/cli.js dev -c "node ./server.js"`, - }, - dependencies: { - "@remix-run/css-bundle": "0.0.0-local-version", - "@remix-run/node": "0.0.0-local-version", - "@remix-run/react": "0.0.0-local-version", - "cross-env": "0.0.0-local-version", - express: "0.0.0-local-version", - isbot: "0.0.0-local-version", - "postcss-import": "0.0.0-local-version", - react: "0.0.0-local-version", - "react-dom": "0.0.0-local-version", - tailwindcss: "0.0.0-local-version", - }, - devDependencies: { - "@remix-run/dev": "0.0.0-local-version", - "@types/react": "0.0.0-local-version", - "@types/react-dom": "0.0.0-local-version", - typescript: "0.0.0-local-version", - }, - engines: { - node: ">=18.0.0", - }, - }), - - "server.js": js` - import path from "path"; - import url from "url"; - import express from "express"; - import { createRequestHandler } from "@remix-run/express"; - import { broadcastDevReady, installGlobals } from "@remix-run/node"; - - installGlobals(); - - const app = express(); - app.use(express.static("public", { immutable: true, maxAge: "1y" })); - - const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); - - app.all( - "*", - createRequestHandler({ - build: await import(BUILD_PATH), - mode: process.env.NODE_ENV, - }) - ); - - let port = ${options.appPort}; - app.listen(port, async () => { - let build = await import(BUILD_PATH); - console.log('โ app ready: http://localhost:' + port); - if (process.env.NODE_ENV === 'development') { - broadcastDevReady(build); - } - }); - `, - - "postcss.config.cjs": js` +let files = { + "postcss.config.cjs": js` module.exports = { plugins: { "postcss-import": {}, // Testing PostCSS cache invalidation @@ -93,7 +27,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ }; `, - "tailwind.config.js": js` + "tailwind.config.js": js` /** @type {import('tailwindcss').Config} */ export default { content: ["./app/**/*.{ts,tsx,jsx,js}"], @@ -104,45 +38,45 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ }; `, - "app/tailwind.css": css` - @tailwind base; - @tailwind components; - @tailwind utilities; - `, + "app/tailwind.css": css` + @tailwind base; + @tailwind components; + @tailwind utilities; + `, - "app/stylesWithImport.css": css` - @import "./importedStyle.css"; - `, + "app/stylesWithImport.css": css` + @import "./importedStyle.css"; + `, - "app/importedStyle.css": css` - .importedStyle { - font-weight: normal; - } - `, + "app/importedStyle.css": css` + .importedStyle { + font-weight: normal; + } + `, - "app/sideEffectStylesWithImport.css": css` - @import "./importedSideEffectStyle.css"; - `, + "app/sideEffectStylesWithImport.css": css` + @import "./importedSideEffectStyle.css"; + `, - "app/importedSideEffectStyle.css": css` - .importedSideEffectStyle { - font-size: initial; - } - `, + "app/importedSideEffectStyle.css": css` + .importedSideEffectStyle { + font-size: initial; + } + `, - "app/style.module.css": css` - .test { - composes: color from "./composedStyle.module.css"; - } - `, + "app/style.module.css": css` + .test { + composes: color from "./composedStyle.module.css"; + } + `, - "app/composedStyle.module.css": css` - .color { - color: initial; - } - `, + "app/composedStyle.module.css": css` + .color { + color: initial; + } + `, - "app/root.tsx": js` + "app/root.tsx": js` import type { LinksFunction } from "@remix-run/node"; import { Link, Links, LiveReload, Meta, Outlet, Scripts } from "@remix-run/react"; import { cssBundleHref } from "@remix-run/css-bundle"; @@ -192,7 +126,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ } `, - "app/routes/_index.tsx": js` + "app/routes/_index.tsx": js` import { useLoaderData } from "@remix-run/react"; export function shouldRevalidate(args) { return true; @@ -207,7 +141,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ } `, - "app/routes/about.tsx": js` + "app/routes/about.tsx": js` import Counter from "../components/counter"; export default function About() { return ( @@ -218,7 +152,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ ) } `, - "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' + "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' export const loader = () => "crazy" export const Component = () => { const data = useLoaderData() @@ -230,7 +164,7 @@ whatsup `, - "app/components/counter.tsx": js` + "app/components/counter.tsx": js` import * as React from "react"; export default function Counter({ id }) { let [count, setCount] = React.useState(0); @@ -241,50 +175,135 @@ whatsup ); } `, - }, -}); - -let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - -let wait = async ( - callback: () => boolean, - { timeoutMs = 1000, intervalMs = 250 } = {} -) => { - let start = Date.now(); - while (Date.now() - start <= timeoutMs) { - if (callback()) { - return; - } - await sleep(intervalMs); - } - throw Error(`wait: timeout ${timeoutMs}ms`); }; -let bufferize = (stream: Readable): (() => string) => { - let buffer = ""; - stream.on("data", (data) => (buffer += data.toString())); - return () => buffer; +let packageJson = (options: { devScript: string; deps?: string[] }) => { + return json({ + private: true, + sideEffects: false, + type: "module", + scripts: { + dev: options.devScript, + }, + dependencies: deps([ + ...(options.deps ?? []), + "@remix-run/css-bundle", + "@remix-run/express", + "@remix-run/node", + "@remix-run/react", + "cross-env", + "express", + "isbot", + "postcss-import", + "react", + "react-dom", + "tailwindcss", + ]), + devDependencies: deps([ + "@remix-run/dev", + "@types/react", + "@types/react-dom", + "typescript", + ]), + engines: { + node: ">=18.0.0", + }, + }); }; -let logConsoleError = (error: Error) => { - console.error(`[console] ${error.name}: ${error.message}`); -}; +let customServer = (options: { appPort: number; devReady: string }) => { + return js` + import path from "path"; + import url from "url"; + import express from "express"; + import { createRequestHandler } from "@remix-run/express"; + import { ${options.devReady}, installGlobals } from "@remix-run/node"; -let expectConsoleError = ( - isExpected: (error: Error) => boolean, - unexpected = logConsoleError -) => { - return (error: Error) => { - if (isExpected(error)) { - return; - } - unexpected(error); - }; + installGlobals(); + + const app = express(); + app.use(express.static("public", { immutable: true, maxAge: "1y" })); + + const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); + + app.all( + "*", + createRequestHandler({ + build: await import(BUILD_PATH), + mode: process.env.NODE_ENV, + }) + ); + + let port = ${options.appPort}; + app.listen(port, async () => { + let build = await import(BUILD_PATH); + console.log('โ app ready: http://localhost:' + port); + if (process.env.NODE_ENV === 'development') { + ${options.devReady}(build); + } + }); + `; }; -let HMR_TIMEOUT_MS = 10_000; +let HMR_TIMEOUT_MS = 30_000; + +let remix = "node ./node_modules/@remix-run/dev/dist/cli.js"; +let serve = "node ./node_modules/@remix-run/serve/dist/cli.js"; + +test("HMR for remix-serve", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `cross-env PORT=${appPort} ${remix} dev -c "${serve} ./build/index.js"`, + deps: ["@remix-run/serve"], + }), + }), + appReadyPattern: /\[remix-serve\] /, + }); +}); + +test("HMR for custom server with broadcast", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `${remix} dev -c "node ./server.js"`, + deps: ["@remix-run/express"], + }), + "server.js": customServer({ + appPort, + devReady: "broadcastDevReady", + }), + }), + appReadyPattern: /โ app ready: /, + }); +}); -test("HMR", async ({ page, browserName }) => { +test("HMR for custom server with log", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `${remix} dev -c "node ./server.js"`, + deps: ["@remix-run/express"], + }), + "server.js": customServer({ + appPort, + devReady: "logDevReady", + }), + }), + appReadyPattern: /โ app ready: /, + }); +}); + +async function dev( + page: Page, + options: { + files: (appPort: number) => Record; + appReadyPattern: RegExp; + } +) { // uncomment for debugging // page.on("console", (msg) => console.log(msg.text())); page.on("pageerror", logConsoleError); @@ -296,24 +315,32 @@ test("HMR", async ({ page, browserName }) => { } }); - let portRange = makeRange(3080, 3099); - let appPort = await getPort({ port: portRange }); - let devPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject(fixture({ appPort, devPort })); + let appPort = await getPort(); + let devPort = await getPort(); + + let fixture: FixtureInit = { + config: { + dev: { + port: devPort, + }, + }, + files: options.files(appPort), + }; + + let projectDir = await createFixtureProject(fixture); + + let devProc = execa("npm", ["run", "dev"], { cwd: projectDir }); + let devStdout = bufferize(devProc.stdout!); + let devStderr = bufferize(devProc.stderr!); - // spin up dev server - let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); - let devStdout = bufferize(dev.stdout!); - let devStderr = bufferize(dev.stderr!); try { await wait( () => { - if (dev.exitCode) throw Error("Dev server exited early"); - return /โ app ready: /.test(devStdout()); + if (devProc.exitCode) throw Error("Dev server exited early"); + return options.appReadyPattern.test(devStdout()); }, { timeoutMs: HMR_TIMEOUT_MS } ); - await page.goto(`http://localhost:${appPort}`, { waitUntil: "networkidle", }); @@ -590,7 +617,7 @@ whatsup // let expectedErrorCount = 0; let expectDestructureTypeError = expectConsoleError((error) => { let expectedMessage = new Set([ - // chrome, edge + // chrome, msedge "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", // firefox "(intermediate value)() is null", @@ -636,6 +663,105 @@ whatsup console.log("stderr end -------------------------"); throw e; } finally { - dev.kill(); + devProc.pid && (await killtree(devProc.pid)); } -}); +} + +let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +let wait = async ( + callback: () => boolean, + { timeoutMs = 1000, intervalMs = 250 } = {} +) => { + let start = Date.now(); + while (Date.now() - start <= timeoutMs) { + if (callback()) { + return; + } + await sleep(intervalMs); + } + throw Error(`wait: timeout ${timeoutMs}ms`); +}; + +let bufferize = (stream: Readable): (() => string) => { + let buffer = ""; + stream.on("data", (data) => (buffer += data.toString())); + return () => buffer; +}; + +let logConsoleError = (error: Error) => { + console.error(`[console] ${error.name}: ${error.message}`); +}; + +let expectConsoleError = ( + isExpected: (error: Error) => boolean, + unexpected = logConsoleError +) => { + return (error: Error) => { + if (isExpected(error)) { + return; + } + unexpected(error); + }; +}; + +let deps = (packages: string[]): Record => { + return Object.fromEntries( + packages.map((pkg) => [pkg, "0.0.0-local-version"]) + ); +}; + +let isWindows = process.platform === "win32"; + +let kill = async (pid: number) => { + if (!isAlive(pid)) return; + if (isWindows) { + await execa("taskkill", ["/F", "/PID", pid.toString()]).catch((error) => { + // taskkill 128 -> the process is already dead + if (error.exitCode === 128) return; + if (/There is no running instance of the task./.test(error.message)) + return; + console.warn(error.message); + }); + return; + } + await execa("kill", ["-9", pid.toString()]).catch((error) => { + // process is already dead + if (/No such process/.test(error.message)) return; + console.warn(error.message); + }); +}; + +let isAlive = (pid: number) => { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return false; + } +}; + +let killtree = async (pid: number) => { + let descendants = await pidtree(pid).catch(() => undefined); + if (descendants === undefined) return; + let pids = [pid, ...descendants]; + + await Promise.all(pids.map(kill)); + + return new Promise((resolve, reject) => { + let check = setInterval(() => { + pids = pids.filter(isAlive); + if (pids.length === 0) { + clearInterval(check); + resolve(); + } + }, 50); + + setTimeout(() => { + clearInterval(check); + reject( + new Error("Timeout: Processes did not exit within the specified time.") + ); + }, 2000); + }); +}; diff --git a/integration/playwright.config.ts b/integration/playwright.config.ts index 8b30808720e..62c3e39cfd3 100644 --- a/integration/playwright.config.ts +++ b/integration/playwright.config.ts @@ -26,8 +26,13 @@ const config: PlaywrightTestConfig = { use: devices["Desktop Safari"], }, { - name: "edge", - use: devices["Desktop Edge"], + name: "msedge", + use: { + ...devices["Desktop Edge"], + // Desktop Edge uses chromium by default + // https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/deviceDescriptorsSource.json#L1502 + channel: "msedge", + }, }, { name: "firefox", diff --git a/package.json b/package.json index b895b422f02..aa73cead05d 100644 --- a/package.json +++ b/package.json @@ -97,8 +97,10 @@ "eslint": "^8.23.1", "eslint-plugin-markdown": "^2.2.1", "eslint-plugin-prefer-let": "^3.0.1", + "execa": "5.1.1", "express": "^4.17.1", "front-matter": "^4.0.2", + "get-port": "5.1.1", "glob": "8.0.3", "isbot": "^3.5.1", "jest": "^27.5.1", @@ -109,6 +111,7 @@ "mime": "^3.0.0", "npm-run-all": "^4.1.5", "patch-package": "^6.5.0", + "pidtree": "^0.6.0", "postcss-import": "^15.1.0", "prettier": "^2.7.1", "prompt-confirm": "^2.0.4", diff --git a/packages/remix-node/__tests__/sessions-test.ts b/packages/remix-node/__tests__/sessions-test.ts index b10b33bda93..5aa8c24f52a 100644 --- a/packages/remix-node/__tests__/sessions-test.ts +++ b/packages/remix-node/__tests__/sessions-test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { promises as fsp } from "node:fs"; import os from "node:os"; -import { createFileSessionStorage } from "../sessions/fileStorage"; +import { createFileSessionStorage, getFile } from "../sessions/fileStorage"; function getCookieFromSetCookie(setCookie: string): string { return setCookie.split(/;\s*/)[0]; @@ -62,6 +62,39 @@ describe("File session storage", () => { await expect(destroySession(session)).resolves.not.toThrowError(); }); + it("saves expires to file if expires provided to commitSession when creating new cookie", async () => { + let { getSession, commitSession } = createFileSessionStorage({ + dir, + cookie: { secrets: ["secret1"] }, + }); + let session = await getSession(); + session.set("user", "mjackson"); + let date = new Date(Date.now() + 1000 * 60); + let cookieHeader = await commitSession(session, { expires: date }); + let createdSession = await getSession(cookieHeader); + + let { id } = createdSession; + let fileContents = await fsp.readFile(getFile(dir, id), "utf8"); + let fileData = JSON.parse(fileContents); + expect(fileData.expires).toEqual(date.toISOString()); + }); + + it("saves expires to file if maxAge provided to commitSession when creating new cookie", async () => { + let { getSession, commitSession } = createFileSessionStorage({ + dir, + cookie: { secrets: ["secret1"] }, + }); + let session = await getSession(); + session.set("user", "mjackson"); + let cookieHeader = await commitSession(session, { maxAge: 60 }); + let createdSession = await getSession(cookieHeader); + + let { id } = createdSession; + let fileContents = await fsp.readFile(getFile(dir, id), "utf8"); + let fileData = JSON.parse(fileContents); + expect(typeof fileData.expires).toBe("string"); + }); + describe("when a new secret shows up in the rotation", () => { it("unsigns old session cookies using the old secret and encodes new cookies using the new secret", async () => { let { getSession, commitSession } = createFileSessionStorage({ diff --git a/packages/remix-node/sessions/fileStorage.ts b/packages/remix-node/sessions/fileStorage.ts index a7eedf3b8cb..042539710cf 100644 --- a/packages/remix-node/sessions/fileStorage.ts +++ b/packages/remix-node/sessions/fileStorage.ts @@ -103,7 +103,7 @@ export function createFileSessionStorage({ }); } -function getFile(dir: string, id: string): string { +export function getFile(dir: string, id: string): string { // Divide the session id up into a directory (first 2 bytes) and filename // (remaining 6 bytes) to reduce the chance of having very large directories, // which should speed up file access. This is a maximum of 2^16 directories, diff --git a/packages/remix-serve/cli.ts b/packages/remix-serve/cli.ts index 97dd18dd33a..f1ffe8b3791 100644 --- a/packages/remix-serve/cli.ts +++ b/packages/remix-serve/cli.ts @@ -1,13 +1,15 @@ import "@remix-run/node/install"; -import path from "node:path"; +import fs from "node:fs"; import os from "node:os"; +import path from "node:path"; import url from "node:url"; import { type ServerBuild, broadcastDevReady, installGlobals, } from "@remix-run/node"; -import { createRequestHandler } from "@remix-run/express"; +import { type RequestHandler, createRequestHandler } from "@remix-run/express"; +import chokidar from "chokidar"; import compression from "compression"; import express from "express"; import morgan from "morgan"; @@ -32,11 +34,43 @@ async function run() { process.exit(1); } - let buildPath = url.pathToFileURL( - path.resolve(process.cwd(), buildPathArg) - ).href; + let buildPath = path.resolve(buildPathArg); - let build: ServerBuild = await import(buildPath); + async function reimportServer() { + let stat = fs.statSync(buildPath); + + // use a timestamp query parameter to bust the import cache + return import(url.pathToFileURL(buildPath).href + "?t=" + stat.mtimeMs); + } + + function createDevRequestHandler(initialBuild: ServerBuild): RequestHandler { + let build = initialBuild; + async function handleServerUpdate() { + // 1. re-import the server build + build = await reimportServer(); + // 2. tell Remix that this app server is now up-to-date and ready + broadcastDevReady(build); + } + + chokidar + .watch(buildPath, { ignoreInitial: true }) + .on("add", handleServerUpdate) + .on("change", handleServerUpdate); + + // wrap request handler to make sure its recreated with the latest build for every request + return async (req, res, next) => { + try { + return createRequestHandler({ + build, + mode: "development", + })(req, res, next); + } catch (error) { + next(error); + } + }; + } + + let build: ServerBuild = await reimportServer(); let onListen = () => { let address = @@ -47,10 +81,10 @@ async function run() { ?.address; if (!address) { - console.log(`Remix App Server started at http://localhost:${port}`); + console.log(`[remix-serve] http://localhost:${port}`); } else { console.log( - `Remix App Server started at http://localhost:${port} (http://${address}:${port})` + `[remix-serve] http://localhost:${port} (http://${address}:${port})` ); } if (process.env.NODE_ENV === "development") { @@ -71,22 +105,15 @@ async function run() { app.use(express.static("public", { maxAge: "1h" })); app.use(morgan("tiny")); - let requestHandler: ReturnType | undefined; - app.all("*", async (req, res, next) => { - try { - if (!requestHandler) { - let build = await import(buildPath); - requestHandler = createRequestHandler({ + app.all( + "*", + process.env.NODE_ENV === "development" + ? createDevRequestHandler(build) + : createRequestHandler({ build, mode: process.env.NODE_ENV, - }); - } - - return await requestHandler(req, res, next); - } catch (error) { - next(error); - } - }); + }) + ); let server = process.env.HOST ? app.listen(port, process.env.HOST, onListen) diff --git a/packages/remix-serve/package.json b/packages/remix-serve/package.json index ac6d7552c31..06524c6b710 100644 --- a/packages/remix-serve/package.json +++ b/packages/remix-serve/package.json @@ -17,6 +17,7 @@ "dependencies": { "@remix-run/express": "1.19.3", "@remix-run/node": "1.19.3", + "chokidar": "^3.5.3", "compression": "^1.7.4", "express": "^4.17.1", "morgan": "^1.10.0", diff --git a/packages/remix-server-runtime/sessions.ts b/packages/remix-server-runtime/sessions.ts index bf809b65414..2d4a6c90806 100644 --- a/packages/remix-server-runtime/sessions.ts +++ b/packages/remix-server-runtime/sessions.ts @@ -277,11 +277,17 @@ export const createSessionStorageFactory = }, async commitSession(session, options) { let { id, data } = session; + let expires = + options?.maxAge != null + ? new Date(Date.now() + options.maxAge * 1000) + : options?.expires != null + ? options.expires + : cookie.expires; if (id) { - await updateData(id, data, cookie.expires); + await updateData(id, data, expires); } else { - id = await createData(data, cookie.expires); + id = await createData(data, expires); } return cookie.serialize(id, options); diff --git a/templates/remix-javascript/package.json b/templates/remix-javascript/package.json index 7143725f8e2..521a8e35ae7 100644 --- a/templates/remix-javascript/package.json +++ b/templates/remix-javascript/package.json @@ -4,7 +4,7 @@ "type": "module", "scripts": { "build": "remix build", - "dev": "remix dev", + "dev": "remix dev --manual", "start": "remix-serve build" }, "dependencies": { diff --git a/templates/remix/package.json b/templates/remix/package.json index b79f377eec7..22c4b044d8d 100644 --- a/templates/remix/package.json +++ b/templates/remix/package.json @@ -4,7 +4,7 @@ "type": "module", "scripts": { "build": "remix build", - "dev": "remix dev", + "dev": "remix dev --manual", "start": "remix-serve build", "typecheck": "tsc" }, diff --git a/yarn.lock b/yarn.lock index f161055e2d5..30e78f2d70c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6076,9 +6076,9 @@ get-package-type@^0.1.0: resolved "https://registry.npmjs.org/get-package-type/-/get-package-type-0.1.0.tgz" integrity sha512-pjzuKtY64GYfWizNAJ0fr9VqttZkNiK2iS430LtIHzjBEr6bX8Am2zm4sW4Ro5wjWW5cAlRL1qAMTcXbjNAO2Q== -get-port@^5.1.1: +get-port@5.1.1, get-port@^5.1.1: version "5.1.1" - resolved "https://registry.npmjs.org/get-port/-/get-port-5.1.1.tgz" + resolved "https://registry.npmjs.org/get-port/-/get-port-5.1.1.tgz#0469ed07563479de6efb986baf053dcd7d4e3193" integrity sha512-g/Q1aTSDOxFpchXC4i8ZWvxA1lnPqx/JHqcpIw0/LX9T8x/GBbi6YnlN5nhaKIFkT8oFsscUKgDJYxfwfS6QsQ== get-stream@^5.0.0, get-stream@^5.1.0:
Save - { - navigate(-1); - }} - > + navigate(-1)} type="button"> Cancel
but Edge puts it in some weird code editor markup: + // + // "LUNCH" + expect(await app.getHtml()).toContain(LUNCH); }); test("Form can hit an action", async ({ page }) => { @@ -259,7 +262,11 @@ test.describe("useFetcher", () => { method: "post", }), ]); - await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`); + // Check full HTML here - Chromium/Firefox/Webkit seem to render this in + // a but Edge puts it in some weird code editor markup: + // + // "LUNCH" + expect(await app.getHtml()).toContain(CHEESESTEAK); }); }); diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 743dd98b152..c513017ff4c 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -137,7 +137,7 @@ export async function createAppFixture(fixture: Fixture, mode?: ServerMode) { let newChunk = chunk.toString(); stdout += newChunk; let match: RegExpMatchArray | null = stdout.match( - /started at http:\/\/localhost:(\d+)\s/ + /\[remix-serve\] http:\/\/localhost:(\d+)\s/ ); if (match) { clearTimeout(rejectTimeout); @@ -223,6 +223,26 @@ export async function createFixtureProject( path.join(projectDir, "node_modules"), { overwrite: true } ); + // let remixDev = path.join( + // projectDir, + // "node_modules/@remix-run/dev/dist/cli.js" + // ); + // await fse.chmod(remixDev, 0o755); + // await fse.ensureSymlink( + // remixDev, + // path.join(projectDir, "node_modules/.bin/remix") + // ); + // + // let remixServe = path.join( + // projectDir, + // "node_modules/@remix-run/serve/dist/cli.js" + // ); + // await fse.chmod(remixServe, 0o755); + // await fse.ensureSymlink( + // remixServe, + // path.join(projectDir, "node_modules/.bin/remix-serve") + // ); + await writeTestFiles(init, projectDir); // We update the config file *after* writing test files so that tests can provide a custom diff --git a/integration/helpers/node-template/package.json b/integration/helpers/node-template/package.json index e202cff6ca6..05953d0ec46 100644 --- a/integration/helpers/node-template/package.json +++ b/integration/helpers/node-template/package.json @@ -6,7 +6,7 @@ "scripts": { "build": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js build", "dev": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js dev", - "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js build" + "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js ./build/index.js" }, "dependencies": { "@remix-run/node": "0.0.0-local-version", diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts deleted file mode 100644 index 750b893430d..00000000000 --- a/integration/hmr-log-test.ts +++ /dev/null @@ -1,561 +0,0 @@ -import { test, expect } from "@playwright/test"; -import execa from "execa"; -import fs from "node:fs"; -import path from "node:path"; -import type { Readable } from "node:stream"; -import getPort, { makeRange } from "get-port"; - -import type { FixtureInit } from "./helpers/create-fixture.js"; -import { - createFixtureProject, - css, - js, - json, -} from "./helpers/create-fixture.js"; - -test.setTimeout(120_000); - -let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ - config: { - dev: { - port: options.devPort, - }, - }, - files: { - "package.json": json({ - private: true, - sideEffects: false, - type: "module", - scripts: { - dev: `node ./node_modules/@remix-run/dev/dist/cli.js dev -c "node ./server.js"`, - }, - dependencies: { - "@remix-run/css-bundle": "0.0.0-local-version", - "@remix-run/node": "0.0.0-local-version", - "@remix-run/react": "0.0.0-local-version", - "cross-env": "0.0.0-local-version", - express: "0.0.0-local-version", - isbot: "0.0.0-local-version", - react: "0.0.0-local-version", - "react-dom": "0.0.0-local-version", - tailwindcss: "0.0.0-local-version", - }, - devDependencies: { - "@remix-run/dev": "0.0.0-local-version", - "@types/react": "0.0.0-local-version", - "@types/react-dom": "0.0.0-local-version", - typescript: "0.0.0-local-version", - }, - engines: { - node: ">=18.0.0", - }, - }), - - "server.js": js` - import path from "path"; - import url from "url"; - import express from "express"; - import { createRequestHandler } from "@remix-run/express"; - import { logDevReady, installGlobals } from "@remix-run/node"; - - installGlobals(); - - const app = express(); - app.use(express.static("public", { immutable: true, maxAge: "1y" })); - - const MODE = process.env.NODE_ENV; - const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); - - app.all( - "*", - createRequestHandler({ - build: await import(BUILD_PATH), - mode: MODE, - }) - ); - - let port = ${options.appPort}; - app.listen(port, async () => { - let build = await import(BUILD_PATH); - console.log('โ app ready: http://localhost:' + port); - if (process.env.NODE_ENV === 'development') { - logDevReady(build); - } - }); - `, - - "tailwind.config.js": js` - /** @type {import('tailwindcss').Config} */ - module.exports = { - content: ["./app/**/*.{ts,tsx,jsx,js}"], - theme: { - extend: {}, - }, - plugins: [], - }; - `, - - "app/tailwind.css": css` - @tailwind base; - @tailwind components; - @tailwind utilities; - `, - - "app/styles.module.css": css` - .test { - color: initial; - } - `, - - "app/root.tsx": js` - import type { LinksFunction } from "@remix-run/node"; - import { Link, Links, LiveReload, Meta, Outlet, Scripts } from "@remix-run/react"; - import { cssBundleHref } from "@remix-run/css-bundle"; - - import Counter from "./components/counter"; - import styles from "./tailwind.css"; - - export const links: LinksFunction = () => [ - { rel: "stylesheet", href: styles }, - ...cssBundleHref ? [{ rel: "stylesheet", href: cssBundleHref }] : [], - ]; - - // dummy loader to make sure that HDR is granular - export const loader = () => { - return null; - }; - - export default function Root() { - return ( - - - - - - - - Root Input - - - - - Home - About - MDX - - - - - - - - - ); - } - `, - - "app/routes/_index.tsx": js` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - const t = useLoaderData(); - return ( - - Index Title - - ) - } - `, - - "app/routes/about.tsx": js` - import Counter from "../components/counter"; - export default function About() { - return ( - - About Title - - - ) - } - `, - "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' -export const loader = () => "crazy" -export const Component = () => { - const data = useLoaderData() - return {data} -} - -# heyo -whatsup - - -`, - - "app/components/counter.tsx": js` - import * as React from "react"; - export default function Counter({ id }) { - let [count, setCount] = React.useState(0); - return ( - - setCount(count + 1)}>inc {count} - - ); - } - `, - }, -}); - -let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - -let wait = async ( - callback: () => boolean, - { timeoutMs = 1000, intervalMs = 250 } = {} -) => { - let start = Date.now(); - while (Date.now() - start <= timeoutMs) { - if (callback()) { - return; - } - await sleep(intervalMs); - } - throw Error(`wait: timeout ${timeoutMs}ms`); -}; - -let bufferize = (stream: Readable): (() => string) => { - let buffer = ""; - stream.on("data", (data) => (buffer += data.toString())); - return () => buffer; -}; - -let logConsoleError = (error: Error) => { - console.error(`[console] ${error.name}: ${error.message}`); -}; - -let expectConsoleError = ( - isExpected: (error: Error) => boolean, - unexpected = logConsoleError -) => { - return (error: Error) => { - if (isExpected(error)) { - return; - } - unexpected(error); - }; -}; - -let HMR_TIMEOUT_MS = 10_000; - -test("HMR", async ({ page, browserName }) => { - // uncomment for debugging - // page.on("console", (msg) => console.log(msg.text())); - page.on("pageerror", logConsoleError); - let dataRequests = 0; - page.on("request", (request) => { - let url = new URL(request.url()); - if (url.searchParams.has("_data")) { - dataRequests++; - } - }); - - let portRange = makeRange(4080, 4099); - let appPort = await getPort({ port: portRange }); - let devPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject(fixture({ appPort, devPort })); - - // spin up dev server - let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); - let devStdout = bufferize(dev.stdout!); - let devStderr = bufferize(dev.stderr!); - try { - await wait( - () => { - if (dev.exitCode) throw Error("Dev server exited early"); - return /โ app ready: /.test(devStdout()); - }, - { timeoutMs: HMR_TIMEOUT_MS } - ); - - await page.goto(`http://localhost:${appPort}`, { - waitUntil: "networkidle", - }); - - // `` value as page state that - // would be wiped out by a full page refresh - // but should be persisted by hmr - let input = page.getByLabel("Root Input"); - expect(input).toBeVisible(); - await input.type("asdfasdf"); - - let counter = await page.waitForSelector("#root-counter"); - await counter.click(); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - let indexPath = path.join(projectDir, "app", "routes", "_index.tsx"); - let originalIndex = fs.readFileSync(indexPath, "utf8"); - let counterPath = path.join(projectDir, "app", "components", "counter.tsx"); - let originalCounter = fs.readFileSync(counterPath, "utf8"); - let cssModulePath = path.join(projectDir, "app", "styles.module.css"); - let originalCssModule = fs.readFileSync(cssModulePath, "utf8"); - let mdxPath = path.join(projectDir, "app", "routes", "mdx.mdx"); - let originalMdx = fs.readFileSync(mdxPath, "utf8"); - - // make content and style changed to index route - let newCssModule = ` - .test { - background: black; - color: white; - } - `; - fs.writeFileSync(cssModulePath, newCssModule); - - let newIndex = ` - import { useLoaderData } from "@remix-run/react"; - import styles from "~/styles.module.css"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - const t = useLoaderData(); - return ( - - Changed - - ) - } - `; - fs.writeFileSync(indexPath, newIndex); - - // detect HMR'd content and style changes - await page.waitForLoadState("networkidle"); - - let h1 = page.getByText("Changed"); - await h1.waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(h1).toHaveCSS("color", "rgb(255, 255, 255)"); - expect(h1).toHaveCSS("background-color", "rgb(0, 0, 0)"); - - // verify that `` value was persisted (i.e. hmr, not full page refresh) - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // undo change - fs.writeFileSync(indexPath, originalIndex); - fs.writeFileSync(cssModulePath, originalCssModule); - await page.getByText("Index Title").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // We should not have done any revalidation yet as only UI has changed - expect(dataRequests).toBe(0); - - // add loader - let withLoader1 = ` - import { json } from "@remix-run/node"; - import { useLoaderData } from "@remix-run/react"; - - export let loader = () => json({ hello: "world" }); - - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - let { hello } = useLoaderData(); - return ( - - Hello, {hello} - - ) - } - `; - fs.writeFileSync(indexPath, withLoader1); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(1); - await page.waitForLoadState("networkidle"); - - await page.getByText("Hello, world").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - expect(dataRequests).toBe(1); - - let withLoader2 = ` - import { json } from "@remix-run/node"; - import { useLoaderData } from "@remix-run/react"; - - export function loader() { - return json({ hello: "planet" }) - } - - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - let { hello } = useLoaderData(); - return ( - - Hello, {hello} - - ) - } - `; - fs.writeFileSync(indexPath, withLoader2); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(2); - await page.waitForLoadState("networkidle"); - - await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // change shared component - let updatedCounter = ` - import * as React from "react"; - export default function Counter({ id }) { - let [count, setCount] = React.useState(0); - return ( - - setCount(count - 1)}>dec {count} - - ); - } - `; - fs.writeFileSync(counterPath, updatedCounter); - await page.waitForSelector(`#root-counter:has-text("dec 1")`); - counter = await page.waitForSelector("#root-counter"); - await counter.click(); - await counter.click(); - await page.waitForSelector(`#root-counter:has-text("dec -1")`); - - await page.click(`a[href="/about"]`); - let aboutCounter = await page.waitForSelector( - `#about-counter:has-text("dec 0")` - ); - await aboutCounter.click(); - await page.waitForSelector(`#about-counter:has-text("dec -1")`); - - // undo change - fs.writeFileSync(counterPath, originalCounter); - - counter = await page.waitForSelector(`#root-counter:has-text("inc -1")`); - await counter.click(); - counter = await page.waitForSelector(`#root-counter:has-text("inc 0")`); - - aboutCounter = await page.waitForSelector( - `#about-counter:has-text("inc -1")` - ); - await aboutCounter.click(); - aboutCounter = await page.waitForSelector( - `#about-counter:has-text("inc 0")` - ); - - expect(dataRequests).toBe(2); - - // mdx - await page.click(`a[href="/mdx"]`); - await page.waitForSelector(`#crazy`); - let mdx = `import { useLoaderData } from '@remix-run/react' -export const loader = () => "hot" -export const Component = () => { - const data = useLoaderData() - return {data} -} - -# heyo -whatsup - - -`; - fs.writeFileSync(mdxPath, mdx); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(4); - await page.waitForSelector(`#hot`); - - fs.writeFileSync(mdxPath, originalMdx); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(5); - await page.waitForSelector(`#crazy`); - - // dev server doesn't crash when rebuild fails - await page.click(`a[href="/"]`); - await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); - await page.waitForLoadState("networkidle"); - - let stderr = devStderr(); - let withSyntaxError = ` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - eport efault functio Index() { - const t = useLoaderData(); - return ( - - With Syntax Error - - ) - } - `; - fs.writeFileSync(indexPath, withSyntaxError); - await wait( - () => - devStderr() - .replace(stderr, "") - .includes('Expected ";" but found "efault"'), - { - timeoutMs: HMR_TIMEOUT_MS, - } - ); - - // React Router integration w/ React Refresh has a bug where sometimes rerenders happen with old UI and new data - // in this case causing `TypeError: Cannot destructure property`. - // Need to fix that bug, but it only shows a harmless console error in the browser in dev - page.removeListener("pageerror", logConsoleError); - // let expectedErrorCount = 0; - let expectDestructureTypeError = expectConsoleError((error) => { - let expectedMessage = new Set([ - // chrome, edge - "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", - // firefox - "(intermediate value)() is null", - // webkit - "Right side of assignment cannot be destructured", - ]); - let isExpected = - error.name === "TypeError" && expectedMessage.has(error.message); - // if (isExpected) expectedErrorCount += 1; - return isExpected; - }); - page.on("pageerror", expectDestructureTypeError); - - let withFix = ` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - // const t = useLoaderData(); - return ( - - With Fix - - ) - } - `; - fs.writeFileSync(indexPath, withFix); - await page.waitForLoadState("networkidle"); - await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); - - // Restore normal console error handling - page.removeListener("pageerror", expectDestructureTypeError); - // expect(expectedErrorCount).toBe(browserName === "webkit" ? 1 : 2); - page.addListener("pageerror", logConsoleError); - } catch (e) { - console.log("stdout begin -----------------------"); - console.log(devStdout()); - console.log("stdout end -------------------------"); - - console.log("stderr begin -----------------------"); - console.log(devStderr()); - console.log("stderr end -------------------------"); - throw e; - } finally { - dev.kill(); - } -}); diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index 0339e97f5c1..dfec1d43ad8 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -1,9 +1,11 @@ -import { test, expect } from "@playwright/test"; -import execa from "execa"; import fs from "node:fs"; import path from "node:path"; import type { Readable } from "node:stream"; -import getPort, { makeRange } from "get-port"; +import type { Page } from "@playwright/test"; +import { test, expect } from "@playwright/test"; +import execa from "execa"; +import getPort from "get-port"; +import pidtree from "pidtree"; import type { FixtureInit } from "./helpers/create-fixture.js"; import { @@ -15,76 +17,8 @@ import { test.setTimeout(150_000); -let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ - config: { - dev: { - port: options.devPort, - }, - }, - files: { - "package.json": json({ - private: true, - sideEffects: false, - type: "module", - scripts: { - dev: `node ./node_modules/@remix-run/dev/dist/cli.js dev -c "node ./server.js"`, - }, - dependencies: { - "@remix-run/css-bundle": "0.0.0-local-version", - "@remix-run/node": "0.0.0-local-version", - "@remix-run/react": "0.0.0-local-version", - "cross-env": "0.0.0-local-version", - express: "0.0.0-local-version", - isbot: "0.0.0-local-version", - "postcss-import": "0.0.0-local-version", - react: "0.0.0-local-version", - "react-dom": "0.0.0-local-version", - tailwindcss: "0.0.0-local-version", - }, - devDependencies: { - "@remix-run/dev": "0.0.0-local-version", - "@types/react": "0.0.0-local-version", - "@types/react-dom": "0.0.0-local-version", - typescript: "0.0.0-local-version", - }, - engines: { - node: ">=18.0.0", - }, - }), - - "server.js": js` - import path from "path"; - import url from "url"; - import express from "express"; - import { createRequestHandler } from "@remix-run/express"; - import { broadcastDevReady, installGlobals } from "@remix-run/node"; - - installGlobals(); - - const app = express(); - app.use(express.static("public", { immutable: true, maxAge: "1y" })); - - const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); - - app.all( - "*", - createRequestHandler({ - build: await import(BUILD_PATH), - mode: process.env.NODE_ENV, - }) - ); - - let port = ${options.appPort}; - app.listen(port, async () => { - let build = await import(BUILD_PATH); - console.log('โ app ready: http://localhost:' + port); - if (process.env.NODE_ENV === 'development') { - broadcastDevReady(build); - } - }); - `, - - "postcss.config.cjs": js` +let files = { + "postcss.config.cjs": js` module.exports = { plugins: { "postcss-import": {}, // Testing PostCSS cache invalidation @@ -93,7 +27,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ }; `, - "tailwind.config.js": js` + "tailwind.config.js": js` /** @type {import('tailwindcss').Config} */ export default { content: ["./app/**/*.{ts,tsx,jsx,js}"], @@ -104,45 +38,45 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ }; `, - "app/tailwind.css": css` - @tailwind base; - @tailwind components; - @tailwind utilities; - `, + "app/tailwind.css": css` + @tailwind base; + @tailwind components; + @tailwind utilities; + `, - "app/stylesWithImport.css": css` - @import "./importedStyle.css"; - `, + "app/stylesWithImport.css": css` + @import "./importedStyle.css"; + `, - "app/importedStyle.css": css` - .importedStyle { - font-weight: normal; - } - `, + "app/importedStyle.css": css` + .importedStyle { + font-weight: normal; + } + `, - "app/sideEffectStylesWithImport.css": css` - @import "./importedSideEffectStyle.css"; - `, + "app/sideEffectStylesWithImport.css": css` + @import "./importedSideEffectStyle.css"; + `, - "app/importedSideEffectStyle.css": css` - .importedSideEffectStyle { - font-size: initial; - } - `, + "app/importedSideEffectStyle.css": css` + .importedSideEffectStyle { + font-size: initial; + } + `, - "app/style.module.css": css` - .test { - composes: color from "./composedStyle.module.css"; - } - `, + "app/style.module.css": css` + .test { + composes: color from "./composedStyle.module.css"; + } + `, - "app/composedStyle.module.css": css` - .color { - color: initial; - } - `, + "app/composedStyle.module.css": css` + .color { + color: initial; + } + `, - "app/root.tsx": js` + "app/root.tsx": js` import type { LinksFunction } from "@remix-run/node"; import { Link, Links, LiveReload, Meta, Outlet, Scripts } from "@remix-run/react"; import { cssBundleHref } from "@remix-run/css-bundle"; @@ -192,7 +126,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ } `, - "app/routes/_index.tsx": js` + "app/routes/_index.tsx": js` import { useLoaderData } from "@remix-run/react"; export function shouldRevalidate(args) { return true; @@ -207,7 +141,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ } `, - "app/routes/about.tsx": js` + "app/routes/about.tsx": js` import Counter from "../components/counter"; export default function About() { return ( @@ -218,7 +152,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ ) } `, - "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' + "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' export const loader = () => "crazy" export const Component = () => { const data = useLoaderData() @@ -230,7 +164,7 @@ whatsup `, - "app/components/counter.tsx": js` + "app/components/counter.tsx": js` import * as React from "react"; export default function Counter({ id }) { let [count, setCount] = React.useState(0); @@ -241,50 +175,135 @@ whatsup ); } `, - }, -}); - -let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - -let wait = async ( - callback: () => boolean, - { timeoutMs = 1000, intervalMs = 250 } = {} -) => { - let start = Date.now(); - while (Date.now() - start <= timeoutMs) { - if (callback()) { - return; - } - await sleep(intervalMs); - } - throw Error(`wait: timeout ${timeoutMs}ms`); }; -let bufferize = (stream: Readable): (() => string) => { - let buffer = ""; - stream.on("data", (data) => (buffer += data.toString())); - return () => buffer; +let packageJson = (options: { devScript: string; deps?: string[] }) => { + return json({ + private: true, + sideEffects: false, + type: "module", + scripts: { + dev: options.devScript, + }, + dependencies: deps([ + ...(options.deps ?? []), + "@remix-run/css-bundle", + "@remix-run/express", + "@remix-run/node", + "@remix-run/react", + "cross-env", + "express", + "isbot", + "postcss-import", + "react", + "react-dom", + "tailwindcss", + ]), + devDependencies: deps([ + "@remix-run/dev", + "@types/react", + "@types/react-dom", + "typescript", + ]), + engines: { + node: ">=18.0.0", + }, + }); }; -let logConsoleError = (error: Error) => { - console.error(`[console] ${error.name}: ${error.message}`); -}; +let customServer = (options: { appPort: number; devReady: string }) => { + return js` + import path from "path"; + import url from "url"; + import express from "express"; + import { createRequestHandler } from "@remix-run/express"; + import { ${options.devReady}, installGlobals } from "@remix-run/node"; -let expectConsoleError = ( - isExpected: (error: Error) => boolean, - unexpected = logConsoleError -) => { - return (error: Error) => { - if (isExpected(error)) { - return; - } - unexpected(error); - }; + installGlobals(); + + const app = express(); + app.use(express.static("public", { immutable: true, maxAge: "1y" })); + + const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); + + app.all( + "*", + createRequestHandler({ + build: await import(BUILD_PATH), + mode: process.env.NODE_ENV, + }) + ); + + let port = ${options.appPort}; + app.listen(port, async () => { + let build = await import(BUILD_PATH); + console.log('โ app ready: http://localhost:' + port); + if (process.env.NODE_ENV === 'development') { + ${options.devReady}(build); + } + }); + `; }; -let HMR_TIMEOUT_MS = 10_000; +let HMR_TIMEOUT_MS = 30_000; + +let remix = "node ./node_modules/@remix-run/dev/dist/cli.js"; +let serve = "node ./node_modules/@remix-run/serve/dist/cli.js"; + +test("HMR for remix-serve", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `cross-env PORT=${appPort} ${remix} dev -c "${serve} ./build/index.js"`, + deps: ["@remix-run/serve"], + }), + }), + appReadyPattern: /\[remix-serve\] /, + }); +}); + +test("HMR for custom server with broadcast", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `${remix} dev -c "node ./server.js"`, + deps: ["@remix-run/express"], + }), + "server.js": customServer({ + appPort, + devReady: "broadcastDevReady", + }), + }), + appReadyPattern: /โ app ready: /, + }); +}); -test("HMR", async ({ page, browserName }) => { +test("HMR for custom server with log", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `${remix} dev -c "node ./server.js"`, + deps: ["@remix-run/express"], + }), + "server.js": customServer({ + appPort, + devReady: "logDevReady", + }), + }), + appReadyPattern: /โ app ready: /, + }); +}); + +async function dev( + page: Page, + options: { + files: (appPort: number) => Record; + appReadyPattern: RegExp; + } +) { // uncomment for debugging // page.on("console", (msg) => console.log(msg.text())); page.on("pageerror", logConsoleError); @@ -296,24 +315,32 @@ test("HMR", async ({ page, browserName }) => { } }); - let portRange = makeRange(3080, 3099); - let appPort = await getPort({ port: portRange }); - let devPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject(fixture({ appPort, devPort })); + let appPort = await getPort(); + let devPort = await getPort(); + + let fixture: FixtureInit = { + config: { + dev: { + port: devPort, + }, + }, + files: options.files(appPort), + }; + + let projectDir = await createFixtureProject(fixture); + + let devProc = execa("npm", ["run", "dev"], { cwd: projectDir }); + let devStdout = bufferize(devProc.stdout!); + let devStderr = bufferize(devProc.stderr!); - // spin up dev server - let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); - let devStdout = bufferize(dev.stdout!); - let devStderr = bufferize(dev.stderr!); try { await wait( () => { - if (dev.exitCode) throw Error("Dev server exited early"); - return /โ app ready: /.test(devStdout()); + if (devProc.exitCode) throw Error("Dev server exited early"); + return options.appReadyPattern.test(devStdout()); }, { timeoutMs: HMR_TIMEOUT_MS } ); - await page.goto(`http://localhost:${appPort}`, { waitUntil: "networkidle", }); @@ -590,7 +617,7 @@ whatsup // let expectedErrorCount = 0; let expectDestructureTypeError = expectConsoleError((error) => { let expectedMessage = new Set([ - // chrome, edge + // chrome, msedge "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", // firefox "(intermediate value)() is null", @@ -636,6 +663,105 @@ whatsup console.log("stderr end -------------------------"); throw e; } finally { - dev.kill(); + devProc.pid && (await killtree(devProc.pid)); } -}); +} + +let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +let wait = async ( + callback: () => boolean, + { timeoutMs = 1000, intervalMs = 250 } = {} +) => { + let start = Date.now(); + while (Date.now() - start <= timeoutMs) { + if (callback()) { + return; + } + await sleep(intervalMs); + } + throw Error(`wait: timeout ${timeoutMs}ms`); +}; + +let bufferize = (stream: Readable): (() => string) => { + let buffer = ""; + stream.on("data", (data) => (buffer += data.toString())); + return () => buffer; +}; + +let logConsoleError = (error: Error) => { + console.error(`[console] ${error.name}: ${error.message}`); +}; + +let expectConsoleError = ( + isExpected: (error: Error) => boolean, + unexpected = logConsoleError +) => { + return (error: Error) => { + if (isExpected(error)) { + return; + } + unexpected(error); + }; +}; + +let deps = (packages: string[]): Record => { + return Object.fromEntries( + packages.map((pkg) => [pkg, "0.0.0-local-version"]) + ); +}; + +let isWindows = process.platform === "win32"; + +let kill = async (pid: number) => { + if (!isAlive(pid)) return; + if (isWindows) { + await execa("taskkill", ["/F", "/PID", pid.toString()]).catch((error) => { + // taskkill 128 -> the process is already dead + if (error.exitCode === 128) return; + if (/There is no running instance of the task./.test(error.message)) + return; + console.warn(error.message); + }); + return; + } + await execa("kill", ["-9", pid.toString()]).catch((error) => { + // process is already dead + if (/No such process/.test(error.message)) return; + console.warn(error.message); + }); +}; + +let isAlive = (pid: number) => { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return false; + } +}; + +let killtree = async (pid: number) => { + let descendants = await pidtree(pid).catch(() => undefined); + if (descendants === undefined) return; + let pids = [pid, ...descendants]; + + await Promise.all(pids.map(kill)); + + return new Promise((resolve, reject) => { + let check = setInterval(() => { + pids = pids.filter(isAlive); + if (pids.length === 0) { + clearInterval(check); + resolve(); + } + }, 50); + + setTimeout(() => { + clearInterval(check); + reject( + new Error("Timeout: Processes did not exit within the specified time.") + ); + }, 2000); + }); +}; diff --git a/integration/playwright.config.ts b/integration/playwright.config.ts index 8b30808720e..62c3e39cfd3 100644 --- a/integration/playwright.config.ts +++ b/integration/playwright.config.ts @@ -26,8 +26,13 @@ const config: PlaywrightTestConfig = { use: devices["Desktop Safari"], }, { - name: "edge", - use: devices["Desktop Edge"], + name: "msedge", + use: { + ...devices["Desktop Edge"], + // Desktop Edge uses chromium by default + // https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/deviceDescriptorsSource.json#L1502 + channel: "msedge", + }, }, { name: "firefox", diff --git a/package.json b/package.json index b895b422f02..aa73cead05d 100644 --- a/package.json +++ b/package.json @@ -97,8 +97,10 @@ "eslint": "^8.23.1", "eslint-plugin-markdown": "^2.2.1", "eslint-plugin-prefer-let": "^3.0.1", + "execa": "5.1.1", "express": "^4.17.1", "front-matter": "^4.0.2", + "get-port": "5.1.1", "glob": "8.0.3", "isbot": "^3.5.1", "jest": "^27.5.1", @@ -109,6 +111,7 @@ "mime": "^3.0.0", "npm-run-all": "^4.1.5", "patch-package": "^6.5.0", + "pidtree": "^0.6.0", "postcss-import": "^15.1.0", "prettier": "^2.7.1", "prompt-confirm": "^2.0.4", diff --git a/packages/remix-node/__tests__/sessions-test.ts b/packages/remix-node/__tests__/sessions-test.ts index b10b33bda93..5aa8c24f52a 100644 --- a/packages/remix-node/__tests__/sessions-test.ts +++ b/packages/remix-node/__tests__/sessions-test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { promises as fsp } from "node:fs"; import os from "node:os"; -import { createFileSessionStorage } from "../sessions/fileStorage"; +import { createFileSessionStorage, getFile } from "../sessions/fileStorage"; function getCookieFromSetCookie(setCookie: string): string { return setCookie.split(/;\s*/)[0]; @@ -62,6 +62,39 @@ describe("File session storage", () => { await expect(destroySession(session)).resolves.not.toThrowError(); }); + it("saves expires to file if expires provided to commitSession when creating new cookie", async () => { + let { getSession, commitSession } = createFileSessionStorage({ + dir, + cookie: { secrets: ["secret1"] }, + }); + let session = await getSession(); + session.set("user", "mjackson"); + let date = new Date(Date.now() + 1000 * 60); + let cookieHeader = await commitSession(session, { expires: date }); + let createdSession = await getSession(cookieHeader); + + let { id } = createdSession; + let fileContents = await fsp.readFile(getFile(dir, id), "utf8"); + let fileData = JSON.parse(fileContents); + expect(fileData.expires).toEqual(date.toISOString()); + }); + + it("saves expires to file if maxAge provided to commitSession when creating new cookie", async () => { + let { getSession, commitSession } = createFileSessionStorage({ + dir, + cookie: { secrets: ["secret1"] }, + }); + let session = await getSession(); + session.set("user", "mjackson"); + let cookieHeader = await commitSession(session, { maxAge: 60 }); + let createdSession = await getSession(cookieHeader); + + let { id } = createdSession; + let fileContents = await fsp.readFile(getFile(dir, id), "utf8"); + let fileData = JSON.parse(fileContents); + expect(typeof fileData.expires).toBe("string"); + }); + describe("when a new secret shows up in the rotation", () => { it("unsigns old session cookies using the old secret and encodes new cookies using the new secret", async () => { let { getSession, commitSession } = createFileSessionStorage({ diff --git a/packages/remix-node/sessions/fileStorage.ts b/packages/remix-node/sessions/fileStorage.ts index a7eedf3b8cb..042539710cf 100644 --- a/packages/remix-node/sessions/fileStorage.ts +++ b/packages/remix-node/sessions/fileStorage.ts @@ -103,7 +103,7 @@ export function createFileSessionStorage({ }); } -function getFile(dir: string, id: string): string { +export function getFile(dir: string, id: string): string { // Divide the session id up into a directory (first 2 bytes) and filename // (remaining 6 bytes) to reduce the chance of having very large directories, // which should speed up file access. This is a maximum of 2^16 directories, diff --git a/packages/remix-serve/cli.ts b/packages/remix-serve/cli.ts index 97dd18dd33a..f1ffe8b3791 100644 --- a/packages/remix-serve/cli.ts +++ b/packages/remix-serve/cli.ts @@ -1,13 +1,15 @@ import "@remix-run/node/install"; -import path from "node:path"; +import fs from "node:fs"; import os from "node:os"; +import path from "node:path"; import url from "node:url"; import { type ServerBuild, broadcastDevReady, installGlobals, } from "@remix-run/node"; -import { createRequestHandler } from "@remix-run/express"; +import { type RequestHandler, createRequestHandler } from "@remix-run/express"; +import chokidar from "chokidar"; import compression from "compression"; import express from "express"; import morgan from "morgan"; @@ -32,11 +34,43 @@ async function run() { process.exit(1); } - let buildPath = url.pathToFileURL( - path.resolve(process.cwd(), buildPathArg) - ).href; + let buildPath = path.resolve(buildPathArg); - let build: ServerBuild = await import(buildPath); + async function reimportServer() { + let stat = fs.statSync(buildPath); + + // use a timestamp query parameter to bust the import cache + return import(url.pathToFileURL(buildPath).href + "?t=" + stat.mtimeMs); + } + + function createDevRequestHandler(initialBuild: ServerBuild): RequestHandler { + let build = initialBuild; + async function handleServerUpdate() { + // 1. re-import the server build + build = await reimportServer(); + // 2. tell Remix that this app server is now up-to-date and ready + broadcastDevReady(build); + } + + chokidar + .watch(buildPath, { ignoreInitial: true }) + .on("add", handleServerUpdate) + .on("change", handleServerUpdate); + + // wrap request handler to make sure its recreated with the latest build for every request + return async (req, res, next) => { + try { + return createRequestHandler({ + build, + mode: "development", + })(req, res, next); + } catch (error) { + next(error); + } + }; + } + + let build: ServerBuild = await reimportServer(); let onListen = () => { let address = @@ -47,10 +81,10 @@ async function run() { ?.address; if (!address) { - console.log(`Remix App Server started at http://localhost:${port}`); + console.log(`[remix-serve] http://localhost:${port}`); } else { console.log( - `Remix App Server started at http://localhost:${port} (http://${address}:${port})` + `[remix-serve] http://localhost:${port} (http://${address}:${port})` ); } if (process.env.NODE_ENV === "development") { @@ -71,22 +105,15 @@ async function run() { app.use(express.static("public", { maxAge: "1h" })); app.use(morgan("tiny")); - let requestHandler: ReturnType | undefined; - app.all("*", async (req, res, next) => { - try { - if (!requestHandler) { - let build = await import(buildPath); - requestHandler = createRequestHandler({ + app.all( + "*", + process.env.NODE_ENV === "development" + ? createDevRequestHandler(build) + : createRequestHandler({ build, mode: process.env.NODE_ENV, - }); - } - - return await requestHandler(req, res, next); - } catch (error) { - next(error); - } - }); + }) + ); let server = process.env.HOST ? app.listen(port, process.env.HOST, onListen) diff --git a/packages/remix-serve/package.json b/packages/remix-serve/package.json index ac6d7552c31..06524c6b710 100644 --- a/packages/remix-serve/package.json +++ b/packages/remix-serve/package.json @@ -17,6 +17,7 @@ "dependencies": { "@remix-run/express": "1.19.3", "@remix-run/node": "1.19.3", + "chokidar": "^3.5.3", "compression": "^1.7.4", "express": "^4.17.1", "morgan": "^1.10.0", diff --git a/packages/remix-server-runtime/sessions.ts b/packages/remix-server-runtime/sessions.ts index bf809b65414..2d4a6c90806 100644 --- a/packages/remix-server-runtime/sessions.ts +++ b/packages/remix-server-runtime/sessions.ts @@ -277,11 +277,17 @@ export const createSessionStorageFactory = }, async commitSession(session, options) { let { id, data } = session; + let expires = + options?.maxAge != null + ? new Date(Date.now() + options.maxAge * 1000) + : options?.expires != null + ? options.expires + : cookie.expires; if (id) { - await updateData(id, data, cookie.expires); + await updateData(id, data, expires); } else { - id = await createData(data, cookie.expires); + id = await createData(data, expires); } return cookie.serialize(id, options); diff --git a/templates/remix-javascript/package.json b/templates/remix-javascript/package.json index 7143725f8e2..521a8e35ae7 100644 --- a/templates/remix-javascript/package.json +++ b/templates/remix-javascript/package.json @@ -4,7 +4,7 @@ "type": "module", "scripts": { "build": "remix build", - "dev": "remix dev", + "dev": "remix dev --manual", "start": "remix-serve build" }, "dependencies": { diff --git a/templates/remix/package.json b/templates/remix/package.json index b79f377eec7..22c4b044d8d 100644 --- a/templates/remix/package.json +++ b/templates/remix/package.json @@ -4,7 +4,7 @@ "type": "module", "scripts": { "build": "remix build", - "dev": "remix dev", + "dev": "remix dev --manual", "start": "remix-serve build", "typecheck": "tsc" }, diff --git a/yarn.lock b/yarn.lock index f161055e2d5..30e78f2d70c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6076,9 +6076,9 @@ get-package-type@^0.1.0: resolved "https://registry.npmjs.org/get-package-type/-/get-package-type-0.1.0.tgz" integrity sha512-pjzuKtY64GYfWizNAJ0fr9VqttZkNiK2iS430LtIHzjBEr6bX8Am2zm4sW4Ro5wjWW5cAlRL1qAMTcXbjNAO2Q== -get-port@^5.1.1: +get-port@5.1.1, get-port@^5.1.1: version "5.1.1" - resolved "https://registry.npmjs.org/get-port/-/get-port-5.1.1.tgz" + resolved "https://registry.npmjs.org/get-port/-/get-port-5.1.1.tgz#0469ed07563479de6efb986baf053dcd7d4e3193" integrity sha512-g/Q1aTSDOxFpchXC4i8ZWvxA1lnPqx/JHqcpIw0/LX9T8x/GBbi6YnlN5nhaKIFkT8oFsscUKgDJYxfwfS6QsQ== get-stream@^5.0.0, get-stream@^5.1.0:
but Edge puts it in some weird code editor markup: + // + // "LUNCH" + expect(await app.getHtml()).toContain(CHEESESTEAK); }); }); diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 743dd98b152..c513017ff4c 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -137,7 +137,7 @@ export async function createAppFixture(fixture: Fixture, mode?: ServerMode) { let newChunk = chunk.toString(); stdout += newChunk; let match: RegExpMatchArray | null = stdout.match( - /started at http:\/\/localhost:(\d+)\s/ + /\[remix-serve\] http:\/\/localhost:(\d+)\s/ ); if (match) { clearTimeout(rejectTimeout); @@ -223,6 +223,26 @@ export async function createFixtureProject( path.join(projectDir, "node_modules"), { overwrite: true } ); + // let remixDev = path.join( + // projectDir, + // "node_modules/@remix-run/dev/dist/cli.js" + // ); + // await fse.chmod(remixDev, 0o755); + // await fse.ensureSymlink( + // remixDev, + // path.join(projectDir, "node_modules/.bin/remix") + // ); + // + // let remixServe = path.join( + // projectDir, + // "node_modules/@remix-run/serve/dist/cli.js" + // ); + // await fse.chmod(remixServe, 0o755); + // await fse.ensureSymlink( + // remixServe, + // path.join(projectDir, "node_modules/.bin/remix-serve") + // ); + await writeTestFiles(init, projectDir); // We update the config file *after* writing test files so that tests can provide a custom diff --git a/integration/helpers/node-template/package.json b/integration/helpers/node-template/package.json index e202cff6ca6..05953d0ec46 100644 --- a/integration/helpers/node-template/package.json +++ b/integration/helpers/node-template/package.json @@ -6,7 +6,7 @@ "scripts": { "build": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js build", "dev": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js dev", - "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js build" + "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js ./build/index.js" }, "dependencies": { "@remix-run/node": "0.0.0-local-version", diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts deleted file mode 100644 index 750b893430d..00000000000 --- a/integration/hmr-log-test.ts +++ /dev/null @@ -1,561 +0,0 @@ -import { test, expect } from "@playwright/test"; -import execa from "execa"; -import fs from "node:fs"; -import path from "node:path"; -import type { Readable } from "node:stream"; -import getPort, { makeRange } from "get-port"; - -import type { FixtureInit } from "./helpers/create-fixture.js"; -import { - createFixtureProject, - css, - js, - json, -} from "./helpers/create-fixture.js"; - -test.setTimeout(120_000); - -let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ - config: { - dev: { - port: options.devPort, - }, - }, - files: { - "package.json": json({ - private: true, - sideEffects: false, - type: "module", - scripts: { - dev: `node ./node_modules/@remix-run/dev/dist/cli.js dev -c "node ./server.js"`, - }, - dependencies: { - "@remix-run/css-bundle": "0.0.0-local-version", - "@remix-run/node": "0.0.0-local-version", - "@remix-run/react": "0.0.0-local-version", - "cross-env": "0.0.0-local-version", - express: "0.0.0-local-version", - isbot: "0.0.0-local-version", - react: "0.0.0-local-version", - "react-dom": "0.0.0-local-version", - tailwindcss: "0.0.0-local-version", - }, - devDependencies: { - "@remix-run/dev": "0.0.0-local-version", - "@types/react": "0.0.0-local-version", - "@types/react-dom": "0.0.0-local-version", - typescript: "0.0.0-local-version", - }, - engines: { - node: ">=18.0.0", - }, - }), - - "server.js": js` - import path from "path"; - import url from "url"; - import express from "express"; - import { createRequestHandler } from "@remix-run/express"; - import { logDevReady, installGlobals } from "@remix-run/node"; - - installGlobals(); - - const app = express(); - app.use(express.static("public", { immutable: true, maxAge: "1y" })); - - const MODE = process.env.NODE_ENV; - const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); - - app.all( - "*", - createRequestHandler({ - build: await import(BUILD_PATH), - mode: MODE, - }) - ); - - let port = ${options.appPort}; - app.listen(port, async () => { - let build = await import(BUILD_PATH); - console.log('โ app ready: http://localhost:' + port); - if (process.env.NODE_ENV === 'development') { - logDevReady(build); - } - }); - `, - - "tailwind.config.js": js` - /** @type {import('tailwindcss').Config} */ - module.exports = { - content: ["./app/**/*.{ts,tsx,jsx,js}"], - theme: { - extend: {}, - }, - plugins: [], - }; - `, - - "app/tailwind.css": css` - @tailwind base; - @tailwind components; - @tailwind utilities; - `, - - "app/styles.module.css": css` - .test { - color: initial; - } - `, - - "app/root.tsx": js` - import type { LinksFunction } from "@remix-run/node"; - import { Link, Links, LiveReload, Meta, Outlet, Scripts } from "@remix-run/react"; - import { cssBundleHref } from "@remix-run/css-bundle"; - - import Counter from "./components/counter"; - import styles from "./tailwind.css"; - - export const links: LinksFunction = () => [ - { rel: "stylesheet", href: styles }, - ...cssBundleHref ? [{ rel: "stylesheet", href: cssBundleHref }] : [], - ]; - - // dummy loader to make sure that HDR is granular - export const loader = () => { - return null; - }; - - export default function Root() { - return ( - - - - - - - - Root Input - - - - - Home - About - MDX - - - - - - - - - ); - } - `, - - "app/routes/_index.tsx": js` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - const t = useLoaderData(); - return ( - - Index Title - - ) - } - `, - - "app/routes/about.tsx": js` - import Counter from "../components/counter"; - export default function About() { - return ( - - About Title - - - ) - } - `, - "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' -export const loader = () => "crazy" -export const Component = () => { - const data = useLoaderData() - return {data} -} - -# heyo -whatsup - - -`, - - "app/components/counter.tsx": js` - import * as React from "react"; - export default function Counter({ id }) { - let [count, setCount] = React.useState(0); - return ( - - setCount(count + 1)}>inc {count} - - ); - } - `, - }, -}); - -let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - -let wait = async ( - callback: () => boolean, - { timeoutMs = 1000, intervalMs = 250 } = {} -) => { - let start = Date.now(); - while (Date.now() - start <= timeoutMs) { - if (callback()) { - return; - } - await sleep(intervalMs); - } - throw Error(`wait: timeout ${timeoutMs}ms`); -}; - -let bufferize = (stream: Readable): (() => string) => { - let buffer = ""; - stream.on("data", (data) => (buffer += data.toString())); - return () => buffer; -}; - -let logConsoleError = (error: Error) => { - console.error(`[console] ${error.name}: ${error.message}`); -}; - -let expectConsoleError = ( - isExpected: (error: Error) => boolean, - unexpected = logConsoleError -) => { - return (error: Error) => { - if (isExpected(error)) { - return; - } - unexpected(error); - }; -}; - -let HMR_TIMEOUT_MS = 10_000; - -test("HMR", async ({ page, browserName }) => { - // uncomment for debugging - // page.on("console", (msg) => console.log(msg.text())); - page.on("pageerror", logConsoleError); - let dataRequests = 0; - page.on("request", (request) => { - let url = new URL(request.url()); - if (url.searchParams.has("_data")) { - dataRequests++; - } - }); - - let portRange = makeRange(4080, 4099); - let appPort = await getPort({ port: portRange }); - let devPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject(fixture({ appPort, devPort })); - - // spin up dev server - let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); - let devStdout = bufferize(dev.stdout!); - let devStderr = bufferize(dev.stderr!); - try { - await wait( - () => { - if (dev.exitCode) throw Error("Dev server exited early"); - return /โ app ready: /.test(devStdout()); - }, - { timeoutMs: HMR_TIMEOUT_MS } - ); - - await page.goto(`http://localhost:${appPort}`, { - waitUntil: "networkidle", - }); - - // `` value as page state that - // would be wiped out by a full page refresh - // but should be persisted by hmr - let input = page.getByLabel("Root Input"); - expect(input).toBeVisible(); - await input.type("asdfasdf"); - - let counter = await page.waitForSelector("#root-counter"); - await counter.click(); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - let indexPath = path.join(projectDir, "app", "routes", "_index.tsx"); - let originalIndex = fs.readFileSync(indexPath, "utf8"); - let counterPath = path.join(projectDir, "app", "components", "counter.tsx"); - let originalCounter = fs.readFileSync(counterPath, "utf8"); - let cssModulePath = path.join(projectDir, "app", "styles.module.css"); - let originalCssModule = fs.readFileSync(cssModulePath, "utf8"); - let mdxPath = path.join(projectDir, "app", "routes", "mdx.mdx"); - let originalMdx = fs.readFileSync(mdxPath, "utf8"); - - // make content and style changed to index route - let newCssModule = ` - .test { - background: black; - color: white; - } - `; - fs.writeFileSync(cssModulePath, newCssModule); - - let newIndex = ` - import { useLoaderData } from "@remix-run/react"; - import styles from "~/styles.module.css"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - const t = useLoaderData(); - return ( - - Changed - - ) - } - `; - fs.writeFileSync(indexPath, newIndex); - - // detect HMR'd content and style changes - await page.waitForLoadState("networkidle"); - - let h1 = page.getByText("Changed"); - await h1.waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(h1).toHaveCSS("color", "rgb(255, 255, 255)"); - expect(h1).toHaveCSS("background-color", "rgb(0, 0, 0)"); - - // verify that `` value was persisted (i.e. hmr, not full page refresh) - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // undo change - fs.writeFileSync(indexPath, originalIndex); - fs.writeFileSync(cssModulePath, originalCssModule); - await page.getByText("Index Title").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // We should not have done any revalidation yet as only UI has changed - expect(dataRequests).toBe(0); - - // add loader - let withLoader1 = ` - import { json } from "@remix-run/node"; - import { useLoaderData } from "@remix-run/react"; - - export let loader = () => json({ hello: "world" }); - - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - let { hello } = useLoaderData(); - return ( - - Hello, {hello} - - ) - } - `; - fs.writeFileSync(indexPath, withLoader1); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(1); - await page.waitForLoadState("networkidle"); - - await page.getByText("Hello, world").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - expect(dataRequests).toBe(1); - - let withLoader2 = ` - import { json } from "@remix-run/node"; - import { useLoaderData } from "@remix-run/react"; - - export function loader() { - return json({ hello: "planet" }) - } - - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - let { hello } = useLoaderData(); - return ( - - Hello, {hello} - - ) - } - `; - fs.writeFileSync(indexPath, withLoader2); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(2); - await page.waitForLoadState("networkidle"); - - await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); - expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); - await page.waitForSelector(`#root-counter:has-text("inc 1")`); - - // change shared component - let updatedCounter = ` - import * as React from "react"; - export default function Counter({ id }) { - let [count, setCount] = React.useState(0); - return ( - - setCount(count - 1)}>dec {count} - - ); - } - `; - fs.writeFileSync(counterPath, updatedCounter); - await page.waitForSelector(`#root-counter:has-text("dec 1")`); - counter = await page.waitForSelector("#root-counter"); - await counter.click(); - await counter.click(); - await page.waitForSelector(`#root-counter:has-text("dec -1")`); - - await page.click(`a[href="/about"]`); - let aboutCounter = await page.waitForSelector( - `#about-counter:has-text("dec 0")` - ); - await aboutCounter.click(); - await page.waitForSelector(`#about-counter:has-text("dec -1")`); - - // undo change - fs.writeFileSync(counterPath, originalCounter); - - counter = await page.waitForSelector(`#root-counter:has-text("inc -1")`); - await counter.click(); - counter = await page.waitForSelector(`#root-counter:has-text("inc 0")`); - - aboutCounter = await page.waitForSelector( - `#about-counter:has-text("inc -1")` - ); - await aboutCounter.click(); - aboutCounter = await page.waitForSelector( - `#about-counter:has-text("inc 0")` - ); - - expect(dataRequests).toBe(2); - - // mdx - await page.click(`a[href="/mdx"]`); - await page.waitForSelector(`#crazy`); - let mdx = `import { useLoaderData } from '@remix-run/react' -export const loader = () => "hot" -export const Component = () => { - const data = useLoaderData() - return {data} -} - -# heyo -whatsup - - -`; - fs.writeFileSync(mdxPath, mdx); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(4); - await page.waitForSelector(`#hot`); - - fs.writeFileSync(mdxPath, originalMdx); - await expect.poll(() => dataRequests, { timeout: HMR_TIMEOUT_MS }).toBe(5); - await page.waitForSelector(`#crazy`); - - // dev server doesn't crash when rebuild fails - await page.click(`a[href="/"]`); - await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); - await page.waitForLoadState("networkidle"); - - let stderr = devStderr(); - let withSyntaxError = ` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - eport efault functio Index() { - const t = useLoaderData(); - return ( - - With Syntax Error - - ) - } - `; - fs.writeFileSync(indexPath, withSyntaxError); - await wait( - () => - devStderr() - .replace(stderr, "") - .includes('Expected ";" but found "efault"'), - { - timeoutMs: HMR_TIMEOUT_MS, - } - ); - - // React Router integration w/ React Refresh has a bug where sometimes rerenders happen with old UI and new data - // in this case causing `TypeError: Cannot destructure property`. - // Need to fix that bug, but it only shows a harmless console error in the browser in dev - page.removeListener("pageerror", logConsoleError); - // let expectedErrorCount = 0; - let expectDestructureTypeError = expectConsoleError((error) => { - let expectedMessage = new Set([ - // chrome, edge - "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", - // firefox - "(intermediate value)() is null", - // webkit - "Right side of assignment cannot be destructured", - ]); - let isExpected = - error.name === "TypeError" && expectedMessage.has(error.message); - // if (isExpected) expectedErrorCount += 1; - return isExpected; - }); - page.on("pageerror", expectDestructureTypeError); - - let withFix = ` - import { useLoaderData } from "@remix-run/react"; - export function shouldRevalidate(args) { - return true; - } - export default function Index() { - // const t = useLoaderData(); - return ( - - With Fix - - ) - } - `; - fs.writeFileSync(indexPath, withFix); - await page.waitForLoadState("networkidle"); - await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); - - // Restore normal console error handling - page.removeListener("pageerror", expectDestructureTypeError); - // expect(expectedErrorCount).toBe(browserName === "webkit" ? 1 : 2); - page.addListener("pageerror", logConsoleError); - } catch (e) { - console.log("stdout begin -----------------------"); - console.log(devStdout()); - console.log("stdout end -------------------------"); - - console.log("stderr begin -----------------------"); - console.log(devStderr()); - console.log("stderr end -------------------------"); - throw e; - } finally { - dev.kill(); - } -}); diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index 0339e97f5c1..dfec1d43ad8 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -1,9 +1,11 @@ -import { test, expect } from "@playwright/test"; -import execa from "execa"; import fs from "node:fs"; import path from "node:path"; import type { Readable } from "node:stream"; -import getPort, { makeRange } from "get-port"; +import type { Page } from "@playwright/test"; +import { test, expect } from "@playwright/test"; +import execa from "execa"; +import getPort from "get-port"; +import pidtree from "pidtree"; import type { FixtureInit } from "./helpers/create-fixture.js"; import { @@ -15,76 +17,8 @@ import { test.setTimeout(150_000); -let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ - config: { - dev: { - port: options.devPort, - }, - }, - files: { - "package.json": json({ - private: true, - sideEffects: false, - type: "module", - scripts: { - dev: `node ./node_modules/@remix-run/dev/dist/cli.js dev -c "node ./server.js"`, - }, - dependencies: { - "@remix-run/css-bundle": "0.0.0-local-version", - "@remix-run/node": "0.0.0-local-version", - "@remix-run/react": "0.0.0-local-version", - "cross-env": "0.0.0-local-version", - express: "0.0.0-local-version", - isbot: "0.0.0-local-version", - "postcss-import": "0.0.0-local-version", - react: "0.0.0-local-version", - "react-dom": "0.0.0-local-version", - tailwindcss: "0.0.0-local-version", - }, - devDependencies: { - "@remix-run/dev": "0.0.0-local-version", - "@types/react": "0.0.0-local-version", - "@types/react-dom": "0.0.0-local-version", - typescript: "0.0.0-local-version", - }, - engines: { - node: ">=18.0.0", - }, - }), - - "server.js": js` - import path from "path"; - import url from "url"; - import express from "express"; - import { createRequestHandler } from "@remix-run/express"; - import { broadcastDevReady, installGlobals } from "@remix-run/node"; - - installGlobals(); - - const app = express(); - app.use(express.static("public", { immutable: true, maxAge: "1y" })); - - const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); - - app.all( - "*", - createRequestHandler({ - build: await import(BUILD_PATH), - mode: process.env.NODE_ENV, - }) - ); - - let port = ${options.appPort}; - app.listen(port, async () => { - let build = await import(BUILD_PATH); - console.log('โ app ready: http://localhost:' + port); - if (process.env.NODE_ENV === 'development') { - broadcastDevReady(build); - } - }); - `, - - "postcss.config.cjs": js` +let files = { + "postcss.config.cjs": js` module.exports = { plugins: { "postcss-import": {}, // Testing PostCSS cache invalidation @@ -93,7 +27,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ }; `, - "tailwind.config.js": js` + "tailwind.config.js": js` /** @type {import('tailwindcss').Config} */ export default { content: ["./app/**/*.{ts,tsx,jsx,js}"], @@ -104,45 +38,45 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ }; `, - "app/tailwind.css": css` - @tailwind base; - @tailwind components; - @tailwind utilities; - `, + "app/tailwind.css": css` + @tailwind base; + @tailwind components; + @tailwind utilities; + `, - "app/stylesWithImport.css": css` - @import "./importedStyle.css"; - `, + "app/stylesWithImport.css": css` + @import "./importedStyle.css"; + `, - "app/importedStyle.css": css` - .importedStyle { - font-weight: normal; - } - `, + "app/importedStyle.css": css` + .importedStyle { + font-weight: normal; + } + `, - "app/sideEffectStylesWithImport.css": css` - @import "./importedSideEffectStyle.css"; - `, + "app/sideEffectStylesWithImport.css": css` + @import "./importedSideEffectStyle.css"; + `, - "app/importedSideEffectStyle.css": css` - .importedSideEffectStyle { - font-size: initial; - } - `, + "app/importedSideEffectStyle.css": css` + .importedSideEffectStyle { + font-size: initial; + } + `, - "app/style.module.css": css` - .test { - composes: color from "./composedStyle.module.css"; - } - `, + "app/style.module.css": css` + .test { + composes: color from "./composedStyle.module.css"; + } + `, - "app/composedStyle.module.css": css` - .color { - color: initial; - } - `, + "app/composedStyle.module.css": css` + .color { + color: initial; + } + `, - "app/root.tsx": js` + "app/root.tsx": js` import type { LinksFunction } from "@remix-run/node"; import { Link, Links, LiveReload, Meta, Outlet, Scripts } from "@remix-run/react"; import { cssBundleHref } from "@remix-run/css-bundle"; @@ -192,7 +126,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ } `, - "app/routes/_index.tsx": js` + "app/routes/_index.tsx": js` import { useLoaderData } from "@remix-run/react"; export function shouldRevalidate(args) { return true; @@ -207,7 +141,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ } `, - "app/routes/about.tsx": js` + "app/routes/about.tsx": js` import Counter from "../components/counter"; export default function About() { return ( @@ -218,7 +152,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ ) } `, - "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' + "app/routes/mdx.mdx": `import { useLoaderData } from '@remix-run/react' export const loader = () => "crazy" export const Component = () => { const data = useLoaderData() @@ -230,7 +164,7 @@ whatsup `, - "app/components/counter.tsx": js` + "app/components/counter.tsx": js` import * as React from "react"; export default function Counter({ id }) { let [count, setCount] = React.useState(0); @@ -241,50 +175,135 @@ whatsup ); } `, - }, -}); - -let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - -let wait = async ( - callback: () => boolean, - { timeoutMs = 1000, intervalMs = 250 } = {} -) => { - let start = Date.now(); - while (Date.now() - start <= timeoutMs) { - if (callback()) { - return; - } - await sleep(intervalMs); - } - throw Error(`wait: timeout ${timeoutMs}ms`); }; -let bufferize = (stream: Readable): (() => string) => { - let buffer = ""; - stream.on("data", (data) => (buffer += data.toString())); - return () => buffer; +let packageJson = (options: { devScript: string; deps?: string[] }) => { + return json({ + private: true, + sideEffects: false, + type: "module", + scripts: { + dev: options.devScript, + }, + dependencies: deps([ + ...(options.deps ?? []), + "@remix-run/css-bundle", + "@remix-run/express", + "@remix-run/node", + "@remix-run/react", + "cross-env", + "express", + "isbot", + "postcss-import", + "react", + "react-dom", + "tailwindcss", + ]), + devDependencies: deps([ + "@remix-run/dev", + "@types/react", + "@types/react-dom", + "typescript", + ]), + engines: { + node: ">=18.0.0", + }, + }); }; -let logConsoleError = (error: Error) => { - console.error(`[console] ${error.name}: ${error.message}`); -}; +let customServer = (options: { appPort: number; devReady: string }) => { + return js` + import path from "path"; + import url from "url"; + import express from "express"; + import { createRequestHandler } from "@remix-run/express"; + import { ${options.devReady}, installGlobals } from "@remix-run/node"; -let expectConsoleError = ( - isExpected: (error: Error) => boolean, - unexpected = logConsoleError -) => { - return (error: Error) => { - if (isExpected(error)) { - return; - } - unexpected(error); - }; + installGlobals(); + + const app = express(); + app.use(express.static("public", { immutable: true, maxAge: "1y" })); + + const BUILD_PATH = url.pathToFileURL(path.join(process.cwd(), "build", "index.js")); + + app.all( + "*", + createRequestHandler({ + build: await import(BUILD_PATH), + mode: process.env.NODE_ENV, + }) + ); + + let port = ${options.appPort}; + app.listen(port, async () => { + let build = await import(BUILD_PATH); + console.log('โ app ready: http://localhost:' + port); + if (process.env.NODE_ENV === 'development') { + ${options.devReady}(build); + } + }); + `; }; -let HMR_TIMEOUT_MS = 10_000; +let HMR_TIMEOUT_MS = 30_000; + +let remix = "node ./node_modules/@remix-run/dev/dist/cli.js"; +let serve = "node ./node_modules/@remix-run/serve/dist/cli.js"; + +test("HMR for remix-serve", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `cross-env PORT=${appPort} ${remix} dev -c "${serve} ./build/index.js"`, + deps: ["@remix-run/serve"], + }), + }), + appReadyPattern: /\[remix-serve\] /, + }); +}); + +test("HMR for custom server with broadcast", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `${remix} dev -c "node ./server.js"`, + deps: ["@remix-run/express"], + }), + "server.js": customServer({ + appPort, + devReady: "broadcastDevReady", + }), + }), + appReadyPattern: /โ app ready: /, + }); +}); -test("HMR", async ({ page, browserName }) => { +test("HMR for custom server with log", async ({ page }) => { + await dev(page, { + files: (appPort) => ({ + ...files, + "package.json": packageJson({ + devScript: `${remix} dev -c "node ./server.js"`, + deps: ["@remix-run/express"], + }), + "server.js": customServer({ + appPort, + devReady: "logDevReady", + }), + }), + appReadyPattern: /โ app ready: /, + }); +}); + +async function dev( + page: Page, + options: { + files: (appPort: number) => Record; + appReadyPattern: RegExp; + } +) { // uncomment for debugging // page.on("console", (msg) => console.log(msg.text())); page.on("pageerror", logConsoleError); @@ -296,24 +315,32 @@ test("HMR", async ({ page, browserName }) => { } }); - let portRange = makeRange(3080, 3099); - let appPort = await getPort({ port: portRange }); - let devPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject(fixture({ appPort, devPort })); + let appPort = await getPort(); + let devPort = await getPort(); + + let fixture: FixtureInit = { + config: { + dev: { + port: devPort, + }, + }, + files: options.files(appPort), + }; + + let projectDir = await createFixtureProject(fixture); + + let devProc = execa("npm", ["run", "dev"], { cwd: projectDir }); + let devStdout = bufferize(devProc.stdout!); + let devStderr = bufferize(devProc.stderr!); - // spin up dev server - let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); - let devStdout = bufferize(dev.stdout!); - let devStderr = bufferize(dev.stderr!); try { await wait( () => { - if (dev.exitCode) throw Error("Dev server exited early"); - return /โ app ready: /.test(devStdout()); + if (devProc.exitCode) throw Error("Dev server exited early"); + return options.appReadyPattern.test(devStdout()); }, { timeoutMs: HMR_TIMEOUT_MS } ); - await page.goto(`http://localhost:${appPort}`, { waitUntil: "networkidle", }); @@ -590,7 +617,7 @@ whatsup // let expectedErrorCount = 0; let expectDestructureTypeError = expectConsoleError((error) => { let expectedMessage = new Set([ - // chrome, edge + // chrome, msedge "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", // firefox "(intermediate value)() is null", @@ -636,6 +663,105 @@ whatsup console.log("stderr end -------------------------"); throw e; } finally { - dev.kill(); + devProc.pid && (await killtree(devProc.pid)); } -}); +} + +let sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +let wait = async ( + callback: () => boolean, + { timeoutMs = 1000, intervalMs = 250 } = {} +) => { + let start = Date.now(); + while (Date.now() - start <= timeoutMs) { + if (callback()) { + return; + } + await sleep(intervalMs); + } + throw Error(`wait: timeout ${timeoutMs}ms`); +}; + +let bufferize = (stream: Readable): (() => string) => { + let buffer = ""; + stream.on("data", (data) => (buffer += data.toString())); + return () => buffer; +}; + +let logConsoleError = (error: Error) => { + console.error(`[console] ${error.name}: ${error.message}`); +}; + +let expectConsoleError = ( + isExpected: (error: Error) => boolean, + unexpected = logConsoleError +) => { + return (error: Error) => { + if (isExpected(error)) { + return; + } + unexpected(error); + }; +}; + +let deps = (packages: string[]): Record => { + return Object.fromEntries( + packages.map((pkg) => [pkg, "0.0.0-local-version"]) + ); +}; + +let isWindows = process.platform === "win32"; + +let kill = async (pid: number) => { + if (!isAlive(pid)) return; + if (isWindows) { + await execa("taskkill", ["/F", "/PID", pid.toString()]).catch((error) => { + // taskkill 128 -> the process is already dead + if (error.exitCode === 128) return; + if (/There is no running instance of the task./.test(error.message)) + return; + console.warn(error.message); + }); + return; + } + await execa("kill", ["-9", pid.toString()]).catch((error) => { + // process is already dead + if (/No such process/.test(error.message)) return; + console.warn(error.message); + }); +}; + +let isAlive = (pid: number) => { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return false; + } +}; + +let killtree = async (pid: number) => { + let descendants = await pidtree(pid).catch(() => undefined); + if (descendants === undefined) return; + let pids = [pid, ...descendants]; + + await Promise.all(pids.map(kill)); + + return new Promise((resolve, reject) => { + let check = setInterval(() => { + pids = pids.filter(isAlive); + if (pids.length === 0) { + clearInterval(check); + resolve(); + } + }, 50); + + setTimeout(() => { + clearInterval(check); + reject( + new Error("Timeout: Processes did not exit within the specified time.") + ); + }, 2000); + }); +}; diff --git a/integration/playwright.config.ts b/integration/playwright.config.ts index 8b30808720e..62c3e39cfd3 100644 --- a/integration/playwright.config.ts +++ b/integration/playwright.config.ts @@ -26,8 +26,13 @@ const config: PlaywrightTestConfig = { use: devices["Desktop Safari"], }, { - name: "edge", - use: devices["Desktop Edge"], + name: "msedge", + use: { + ...devices["Desktop Edge"], + // Desktop Edge uses chromium by default + // https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/deviceDescriptorsSource.json#L1502 + channel: "msedge", + }, }, { name: "firefox", diff --git a/package.json b/package.json index b895b422f02..aa73cead05d 100644 --- a/package.json +++ b/package.json @@ -97,8 +97,10 @@ "eslint": "^8.23.1", "eslint-plugin-markdown": "^2.2.1", "eslint-plugin-prefer-let": "^3.0.1", + "execa": "5.1.1", "express": "^4.17.1", "front-matter": "^4.0.2", + "get-port": "5.1.1", "glob": "8.0.3", "isbot": "^3.5.1", "jest": "^27.5.1", @@ -109,6 +111,7 @@ "mime": "^3.0.0", "npm-run-all": "^4.1.5", "patch-package": "^6.5.0", + "pidtree": "^0.6.0", "postcss-import": "^15.1.0", "prettier": "^2.7.1", "prompt-confirm": "^2.0.4", diff --git a/packages/remix-node/__tests__/sessions-test.ts b/packages/remix-node/__tests__/sessions-test.ts index b10b33bda93..5aa8c24f52a 100644 --- a/packages/remix-node/__tests__/sessions-test.ts +++ b/packages/remix-node/__tests__/sessions-test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { promises as fsp } from "node:fs"; import os from "node:os"; -import { createFileSessionStorage } from "../sessions/fileStorage"; +import { createFileSessionStorage, getFile } from "../sessions/fileStorage"; function getCookieFromSetCookie(setCookie: string): string { return setCookie.split(/;\s*/)[0]; @@ -62,6 +62,39 @@ describe("File session storage", () => { await expect(destroySession(session)).resolves.not.toThrowError(); }); + it("saves expires to file if expires provided to commitSession when creating new cookie", async () => { + let { getSession, commitSession } = createFileSessionStorage({ + dir, + cookie: { secrets: ["secret1"] }, + }); + let session = await getSession(); + session.set("user", "mjackson"); + let date = new Date(Date.now() + 1000 * 60); + let cookieHeader = await commitSession(session, { expires: date }); + let createdSession = await getSession(cookieHeader); + + let { id } = createdSession; + let fileContents = await fsp.readFile(getFile(dir, id), "utf8"); + let fileData = JSON.parse(fileContents); + expect(fileData.expires).toEqual(date.toISOString()); + }); + + it("saves expires to file if maxAge provided to commitSession when creating new cookie", async () => { + let { getSession, commitSession } = createFileSessionStorage({ + dir, + cookie: { secrets: ["secret1"] }, + }); + let session = await getSession(); + session.set("user", "mjackson"); + let cookieHeader = await commitSession(session, { maxAge: 60 }); + let createdSession = await getSession(cookieHeader); + + let { id } = createdSession; + let fileContents = await fsp.readFile(getFile(dir, id), "utf8"); + let fileData = JSON.parse(fileContents); + expect(typeof fileData.expires).toBe("string"); + }); + describe("when a new secret shows up in the rotation", () => { it("unsigns old session cookies using the old secret and encodes new cookies using the new secret", async () => { let { getSession, commitSession } = createFileSessionStorage({ diff --git a/packages/remix-node/sessions/fileStorage.ts b/packages/remix-node/sessions/fileStorage.ts index a7eedf3b8cb..042539710cf 100644 --- a/packages/remix-node/sessions/fileStorage.ts +++ b/packages/remix-node/sessions/fileStorage.ts @@ -103,7 +103,7 @@ export function createFileSessionStorage({ }); } -function getFile(dir: string, id: string): string { +export function getFile(dir: string, id: string): string { // Divide the session id up into a directory (first 2 bytes) and filename // (remaining 6 bytes) to reduce the chance of having very large directories, // which should speed up file access. This is a maximum of 2^16 directories, diff --git a/packages/remix-serve/cli.ts b/packages/remix-serve/cli.ts index 97dd18dd33a..f1ffe8b3791 100644 --- a/packages/remix-serve/cli.ts +++ b/packages/remix-serve/cli.ts @@ -1,13 +1,15 @@ import "@remix-run/node/install"; -import path from "node:path"; +import fs from "node:fs"; import os from "node:os"; +import path from "node:path"; import url from "node:url"; import { type ServerBuild, broadcastDevReady, installGlobals, } from "@remix-run/node"; -import { createRequestHandler } from "@remix-run/express"; +import { type RequestHandler, createRequestHandler } from "@remix-run/express"; +import chokidar from "chokidar"; import compression from "compression"; import express from "express"; import morgan from "morgan"; @@ -32,11 +34,43 @@ async function run() { process.exit(1); } - let buildPath = url.pathToFileURL( - path.resolve(process.cwd(), buildPathArg) - ).href; + let buildPath = path.resolve(buildPathArg); - let build: ServerBuild = await import(buildPath); + async function reimportServer() { + let stat = fs.statSync(buildPath); + + // use a timestamp query parameter to bust the import cache + return import(url.pathToFileURL(buildPath).href + "?t=" + stat.mtimeMs); + } + + function createDevRequestHandler(initialBuild: ServerBuild): RequestHandler { + let build = initialBuild; + async function handleServerUpdate() { + // 1. re-import the server build + build = await reimportServer(); + // 2. tell Remix that this app server is now up-to-date and ready + broadcastDevReady(build); + } + + chokidar + .watch(buildPath, { ignoreInitial: true }) + .on("add", handleServerUpdate) + .on("change", handleServerUpdate); + + // wrap request handler to make sure its recreated with the latest build for every request + return async (req, res, next) => { + try { + return createRequestHandler({ + build, + mode: "development", + })(req, res, next); + } catch (error) { + next(error); + } + }; + } + + let build: ServerBuild = await reimportServer(); let onListen = () => { let address = @@ -47,10 +81,10 @@ async function run() { ?.address; if (!address) { - console.log(`Remix App Server started at http://localhost:${port}`); + console.log(`[remix-serve] http://localhost:${port}`); } else { console.log( - `Remix App Server started at http://localhost:${port} (http://${address}:${port})` + `[remix-serve] http://localhost:${port} (http://${address}:${port})` ); } if (process.env.NODE_ENV === "development") { @@ -71,22 +105,15 @@ async function run() { app.use(express.static("public", { maxAge: "1h" })); app.use(morgan("tiny")); - let requestHandler: ReturnType | undefined; - app.all("*", async (req, res, next) => { - try { - if (!requestHandler) { - let build = await import(buildPath); - requestHandler = createRequestHandler({ + app.all( + "*", + process.env.NODE_ENV === "development" + ? createDevRequestHandler(build) + : createRequestHandler({ build, mode: process.env.NODE_ENV, - }); - } - - return await requestHandler(req, res, next); - } catch (error) { - next(error); - } - }); + }) + ); let server = process.env.HOST ? app.listen(port, process.env.HOST, onListen) diff --git a/packages/remix-serve/package.json b/packages/remix-serve/package.json index ac6d7552c31..06524c6b710 100644 --- a/packages/remix-serve/package.json +++ b/packages/remix-serve/package.json @@ -17,6 +17,7 @@ "dependencies": { "@remix-run/express": "1.19.3", "@remix-run/node": "1.19.3", + "chokidar": "^3.5.3", "compression": "^1.7.4", "express": "^4.17.1", "morgan": "^1.10.0", diff --git a/packages/remix-server-runtime/sessions.ts b/packages/remix-server-runtime/sessions.ts index bf809b65414..2d4a6c90806 100644 --- a/packages/remix-server-runtime/sessions.ts +++ b/packages/remix-server-runtime/sessions.ts @@ -277,11 +277,17 @@ export const createSessionStorageFactory = }, async commitSession(session, options) { let { id, data } = session; + let expires = + options?.maxAge != null + ? new Date(Date.now() + options.maxAge * 1000) + : options?.expires != null + ? options.expires + : cookie.expires; if (id) { - await updateData(id, data, cookie.expires); + await updateData(id, data, expires); } else { - id = await createData(data, cookie.expires); + id = await createData(data, expires); } return cookie.serialize(id, options); diff --git a/templates/remix-javascript/package.json b/templates/remix-javascript/package.json index 7143725f8e2..521a8e35ae7 100644 --- a/templates/remix-javascript/package.json +++ b/templates/remix-javascript/package.json @@ -4,7 +4,7 @@ "type": "module", "scripts": { "build": "remix build", - "dev": "remix dev", + "dev": "remix dev --manual", "start": "remix-serve build" }, "dependencies": { diff --git a/templates/remix/package.json b/templates/remix/package.json index b79f377eec7..22c4b044d8d 100644 --- a/templates/remix/package.json +++ b/templates/remix/package.json @@ -4,7 +4,7 @@ "type": "module", "scripts": { "build": "remix build", - "dev": "remix dev", + "dev": "remix dev --manual", "start": "remix-serve build", "typecheck": "tsc" }, diff --git a/yarn.lock b/yarn.lock index f161055e2d5..30e78f2d70c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6076,9 +6076,9 @@ get-package-type@^0.1.0: resolved "https://registry.npmjs.org/get-package-type/-/get-package-type-0.1.0.tgz" integrity sha512-pjzuKtY64GYfWizNAJ0fr9VqttZkNiK2iS430LtIHzjBEr6bX8Am2zm4sW4Ro5wjWW5cAlRL1qAMTcXbjNAO2Q== -get-port@^5.1.1: +get-port@5.1.1, get-port@^5.1.1: version "5.1.1" - resolved "https://registry.npmjs.org/get-port/-/get-port-5.1.1.tgz" + resolved "https://registry.npmjs.org/get-port/-/get-port-5.1.1.tgz#0469ed07563479de6efb986baf053dcd7d4e3193" integrity sha512-g/Q1aTSDOxFpchXC4i8ZWvxA1lnPqx/JHqcpIw0/LX9T8x/GBbi6YnlN5nhaKIFkT8oFsscUKgDJYxfwfS6QsQ== get-stream@^5.0.0, get-stream@^5.1.0:
- setCount(count + 1)}>inc {count} -
- setCount(count - 1)}>dec {count} -