Skip to content

Commit

Permalink
Merge branch 'dev' into defjosiah/act/hmr-promise-router
Browse files Browse the repository at this point in the history
  • Loading branch information
jacob-ebey authored Jul 11, 2023
2 parents 6076545 + 79f9ff3 commit fe8175d
Show file tree
Hide file tree
Showing 22 changed files with 160 additions and 41 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-pears-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/react": patch
---

Narrowed the type of `fetcher.formEncType` to use `FormEncType` from `react-router-dom` instead of `string`
5 changes: 5 additions & 0 deletions .changeset/stylesheet-re-prefetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/react": patch
---

Avoid re-prefetching stylesheets for active routes during a revalidation
4 changes: 2 additions & 2 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ By default, the Remix `rollup` build will strip any `console.debug` calls to avo
REMIX_DEBUG=true yarn watch
```

**`REMIX_LOCAL_BUILD_DIRECTORY`**
**`LOCAL_BUILD_DIRECTORY`**

When developing Remix locally, you often need to go beyond unit/integration tests and test your changes in a local Remix application. The easiest way to do this is to run your local Remix build and use this environment variable to direct `rollup` to write the output files directly into the local Remix application's `node_modules` folder. Then you just need to restart your local Remix application server to pick up the changes.

Expand All @@ -118,7 +118,7 @@ cd my-remix-app
npm run dev

# Tab 2 - remix repository
REMIX_LOCAL_BUILD_DIRECTORY=../my-remix-app yarn watch
LOCAL_BUILD_DIRECTORY=../my-remix-app yarn watch
```

Now - any time you make changes in the Remix repository, they will be written out to the appropriate locations in `../my-remix-app/node_modules` and you can restart the `npm run dev` command to pick them up 🎉.
Expand Down
2 changes: 2 additions & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -554,5 +554,7 @@
- amir-ziaei
- mrkhosravian
- tanerijun
- toufiqnuur
- ally1002
- defjosiah
- AlemTuzlak
17 changes: 15 additions & 2 deletions docs/guides/envvars.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,24 @@ export async function loader() {

If you're using the `@remix-run/cloudflare-pages` adapter, env variables work a little differently. Since Cloudflare Pages are powered by Functions, you'll need to define your local environment variables in the [`.dev.vars`][dev-vars] file. It has the same syntax as `.env` example file mentioned above.

Then, in your `loader` functions, you can access environment variables directly on `context`:
Then, you can pass those through via `getLoadContext` in your server file:

```ts
export const onRequest = createPagesFunctionHandler({
build,
getLoadContext(context) {
// Hand-off Cloudflare ENV vars to the Remix `context` object
return { env: context.env };
},
mode: process.env.NODE_ENV,
});
```

And they'll be available via Remix's `context` in your `loader`/`action` functions:

```tsx
export const loader = async ({ context }: LoaderArgs) => {
console.log(context.SOME_SECRET);
console.log(context.env.SOME_SECRET);
};
```

Expand Down
2 changes: 1 addition & 1 deletion integration/abort-signal-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ test.beforeAll(async () => {
},
});

// This creates an interactive app using puppeteer.
// This creates an interactive app using playwright.
appFixture = await createAppFixture(fixture);
});

Expand Down
2 changes: 1 addition & 1 deletion integration/bug-report-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ test.beforeAll(async () => {
},
});

// This creates an interactive app using puppeteer.
// This creates an interactive app using playwright.
appFixture = await createAppFixture(fixture);
});

Expand Down
4 changes: 2 additions & 2 deletions integration/defer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ test.describe("non-aborted", () => {
},
});

// This creates an interactive app using puppeteer.
// This creates an interactive app using playwright.
appFixture = await createAppFixture(fixture);
});

Expand Down Expand Up @@ -1244,7 +1244,7 @@ test.describe("aborted", () => {
},
});

// This creates an interactive app using puppeteer.
// This creates an interactive app using playwright.
appFixture = await createAppFixture(fixture);
});

Expand Down
92 changes: 92 additions & 0 deletions integration/prefetch-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,3 +342,95 @@ test.describe("prefetch=viewport", () => {
await expect(page.locator("div link")).toHaveCount(0);
});
});

test.describe("other scenarios", () => {
let fixture: Fixture;
let appFixture: AppFixture;

test.afterAll(() => {
appFixture?.close();
});

test("does not add prefetch links for stylesheets already in the DOM (active routes)", async ({
page,
}) => {
fixture = await createFixture({
config: {
future: { v2_routeConvention: true },
},
files: {
"app/root.jsx": js`
import { Links, Meta, Scripts, useFetcher } from "@remix-run/react";
import globalCss from "./global.css";
export function links() {
return [{ rel: "stylesheet", href: globalCss }];
}
export async function action() {
return null;
}
export async function loader() {
return null;
}
export default function Root() {
let fetcher = useFetcher();
return (
<html lang="en">
<head>
<Meta />
<Links />
</head>
<body>
<button
id="submit-fetcher"
onClick={() => fetcher.submit({}, { method: 'post' })}>
Submit Fetcher
</button>
<p id={"fetcher-state--" + fetcher.state}>{fetcher.state}</p>
<Scripts />
</body>
</html>
);
}
`,

"app/global.css": `
body {
background-color: black;
color: white;
}
`,

"app/routes/_index.jsx": js`
export default function() {
return <h2 className="index">Index</h2>;
}
`,
},
});
appFixture = await createAppFixture(fixture);
let requests: { type: string; url: string }[] = [];

page.on("request", (req) => {
requests.push({
type: req.resourceType(),
url: req.url(),
});
});

let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await page.click("#submit-fetcher");
await page.waitForSelector("#fetcher-state--idle");
// We should not send a second request for this root stylesheet that's
// already been rendered in the DOM
let stylesheets = requests.filter(
(r) => r.type === "stylesheet" && /\/global-[a-z0-9]+\.css/i.test(r.url)
);
expect(stylesheets.length).toBe(1);
});
});
2 changes: 1 addition & 1 deletion integration/scroll-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ test.beforeAll(async () => {
},
});

// This creates an interactive app using puppeteer.
// This creates an interactive app using playwright.
appFixture = await createAppFixture(fixture);
});

Expand Down
2 changes: 1 addition & 1 deletion integration/set-cookie-revalidation-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ test.beforeAll(async () => {
},
});

// This creates an interactive app using puppeteer.
// This creates an interactive app using playwright.
appFixture = await createAppFixture(fixture);
});

Expand Down
2 changes: 1 addition & 1 deletion packages/remix-architect/__tests__/server-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from "../server";

// We don't want to test that the remix server works here (that's what the
// puppetteer tests do), we just want to test the architect adapter
// playwright tests do), we just want to test the architect adapter
jest.mock("@remix-run/node", () => {
let original = jest.requireActual("@remix-run/node");
return {
Expand Down
7 changes: 2 additions & 5 deletions packages/remix-dev/compiler/analysis.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fs from "node:fs";
import fs from "fs-extra";
import path from "node:path";
import type { Metafile } from "esbuild";

Expand All @@ -10,8 +10,5 @@ export let writeMetafile = (
metafile: Metafile
) => {
let buildDir = path.dirname(ctx.config.serverBuildPath);
if (!fs.existsSync(buildDir)) {
fs.mkdirSync(buildDir, { recursive: true });
}
fs.writeFileSync(path.join(buildDir, filename), JSON.stringify(metafile));
fs.outputFileSync(path.join(buildDir, filename), JSON.stringify(metafile));
};
13 changes: 9 additions & 4 deletions packages/remix-dev/devServer_unstable/proc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,21 @@ import pidtree from "pidtree";
let isWindows = process.platform === "win32";

export 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) throw error;
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)) throw error;
if (/No such process/.test(error.message)) return;
console.warn(error.message);
});
};

Expand All @@ -34,8 +39,8 @@ export let killtree = async (pid: number) => {

return new Promise<void>((resolve, reject) => {
let check = setInterval(() => {
let alive = pids.filter(isAlive);
if (alive.length === 0) {
pids = pids.filter(isAlive);
if (pids.length === 0) {
clearInterval(check);
resolve();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-express/__tests__/server-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from "../server";

// We don't want to test that the remix server works here (that's what the
// puppetteer tests do), we just want to test the express adapter
// playwright tests do), we just want to test the express adapter
jest.mock("@remix-run/node", () => {
let original = jest.requireActual("@remix-run/node");
return {
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-netlify/__tests__/server-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from "../server";

// We don't want to test that the remix server works here (that's what the
// puppetteer tests do), we just want to test the netlify adapter
// playwright tests do), we just want to test the netlify adapter
jest.mock("@remix-run/node", () => {
let original = jest.requireActual("@remix-run/node");
return {
Expand Down
7 changes: 5 additions & 2 deletions packages/remix-react/links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,12 @@ export async function prefetchStyleLinks(
}
}

// don't block for non-matching media queries
// don't block for non-matching media queries, or for stylesheets that are
// already in the DOM (active route revalidations)
let matchingLinks = styleLinks.filter(
(link) => !link.media || window.matchMedia(link.media).matches
(link) =>
(!link.media || window.matchMedia(link.media).matches) &&
!document.querySelector(`link[rel="stylesheet"][href="${link.href}"]`)
);

await Promise.all(matchingLinks.map(prefetchStyleLink));
Expand Down
10 changes: 5 additions & 5 deletions packages/remix-react/transition.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Location } from "react-router-dom";
import type { Location, FormEncType } from "react-router-dom";

export interface Submission {
action: string;
Expand Down Expand Up @@ -137,7 +137,7 @@ export type FetcherStates<TData = any> = {
type: "actionSubmission";
formMethod: FetcherActionSubmission["method"];
formAction: string;
formEncType: string;
formEncType: FormEncType;
submission: FetcherActionSubmission;
data: TData | undefined;
} & FetcherSubmissionDataTypes;
Expand All @@ -146,7 +146,7 @@ export type FetcherStates<TData = any> = {
type: "loaderSubmission";
formMethod: FetcherLoaderSubmission["method"];
formAction: string;
formEncType: string;
formEncType: FormEncType;
submission: FetcherLoaderSubmission;
data: TData | undefined;
} & FetcherSubmissionDataTypes;
Expand All @@ -155,7 +155,7 @@ export type FetcherStates<TData = any> = {
type: "actionReload";
formMethod: FetcherActionSubmission["method"];
formAction: string;
formEncType: string;
formEncType: FormEncType;
submission: FetcherActionSubmission;
data: TData;
} & FetcherSubmissionDataTypes;
Expand All @@ -164,7 +164,7 @@ export type FetcherStates<TData = any> = {
type: "actionRedirect";
formMethod: FetcherActionSubmission["method"];
formAction: string;
formEncType: string;
formEncType: FormEncType;
submission: FetcherActionSubmission;
data: undefined;
} & FetcherSubmissionDataTypes;
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-vercel/__tests__/server-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from "../server";

// We don't want to test that the remix server works here (that's what the
// Puppeteer tests do), we just want to test the Vercel adapter
// playwright tests do), we just want to test the Vercel adapter
jest.mock("@remix-run/node", () => {
let original = jest.requireActual("@remix-run/node");
return {
Expand Down
2 changes: 1 addition & 1 deletion packages/remix/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let { name: packageName, version } = require("./package.json");
module.exports = function rollup() {
// Don't blow away remix magic exports on local builds, since they've
// already been configured by postinstall
if (process.env.REMIX_LOCAL_BUILD_DIRECTORY) {
if (process.env.LOCAL_BUILD_DIRECTORY) {
return [];
}

Expand Down
11 changes: 4 additions & 7 deletions rollup.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@ const path = require("path");
const REPO_ROOT_DIR = __dirname;

let activeOutputDir = "build";
if (process.env.REMIX_LOCAL_BUILD_DIRECTORY) {
let appDir = path.join(
process.cwd(),
process.env.REMIX_LOCAL_BUILD_DIRECTORY
);
if (process.env.LOCAL_BUILD_DIRECTORY) {
let appDir = path.resolve(process.env.LOCAL_BUILD_DIRECTORY);
try {
fse.readdirSync(path.join(appDir, "node_modules"));
} catch {
console.error(
"Oops! You pointed REMIX_LOCAL_BUILD_DIRECTORY to a directory that " +
"Oops! You pointed LOCAL_BUILD_DIRECTORY to a directory that " +
"does not have a node_modules/ folder. Please `npm install` in that " +
"directory and try again."
);
Expand Down Expand Up @@ -124,7 +121,7 @@ function copyToPlaygrounds() {
await triggerLiveReload(playgroundDir);
}
} else {
// Otherwise, trigger live reload on our REMIX_LOCAL_BUILD_DIRECTORY folder
// Otherwise, trigger live reload on our LOCAL_BUILD_DIRECTORY folder
await triggerLiveReload(activeOutputDir);
}
},
Expand Down
Loading

0 comments on commit fe8175d

Please sign in to comment.