Repository: remix-run/remix
Pull request #5793 (2023-03-14 17:53:13)

feat: bring back old routing and serverBuildTarget deprecation warnings
This reverts commit de546aea65768ae71e22c929e76c215edb531b6b. which brings back deprecation warnings from https://github.com/remix-run/remix/pull/5606 and https://github.com/remix-run/remix/pull/5624

<!--

👋 Hey, thanks for your interest in contributing to Remix!

If this is a simple docs change, go ahead and delete all this.

**Please ask first before starting work on any significant new features.**

It's never a fun experience to have your pull request declined after investing a
lot of time and effort into a new feature. To avoid this from happening, we
request that contributors create a
[Feature Request discussion](https://github.com/remix-run/remix/discussions/new?category=ideas)
to first discuss any significant new features.

https://github.com/remix-run/remix/blob/main/CONTRIBUTING.md

Please fill in or delete each item below:

-->

Closes: #

- [ ] Docs
- [ ] Tests

Testing Strategy:

<!--
Please explain how you tested this. For example:

> This test covers this code: <link to test>

Or

> I opened up my windows machine and ran this script:
>
> ```
> npx create-remix@0.0.0-experimental-7e420ee3 --template remix my-test
> cd my-test
> npm run dev
> ```
-->


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 7b910b825b68b9b24b16b1680b9205f9d5460a3d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/remix-run/remix/new/revert-5790-remove-deprecations?filename=.changeset/fast-jeans-change.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fdev%22%3A%20patch%0A---%0A%0Afeat%3A%20bring%20back%20old%20routing%20and%20serverBuildTarget%20deprecation%20warnings%0A)



Patch:
diff --git a/docs/file-conventions/remix-config.md b/docs/file-conventions/remix-config.md
index 3a975ed16..f2b4e5e54 100644
--- a/docs/file-conventions/remix-config.md
+++ b/docs/file-conventions/remix-config.md
@@ -116,8 +116,7 @@ to `"build/index.js"`.
 
 ## serverBuildTarget
 
-<docs-warning>This option is deprecated and will likely be removed in a future
-stable release. Use a combination of [`publicPath`][public-path],
+<docs-warning>This option is deprecated and will be removed in the next major version release. Use a combination of [`publicPath`][public-path],
 [`serverBuildPath`][server-build-path], [`serverConditions`][server-conditions],
 [`serverDependenciesToBundle`][server-dependencies-to-bundle]
 [`serverMainFields`][server-main-fields], [`serverMinify`][server-minify],
@@ -136,6 +135,18 @@ The `serverBuildTarget` can be one of the following:
 - [`"node-cjs"`][node-cjs]
 - [`"vercel"`][vercel]
 
+**Migration Table:**
+
+| serverBuildTarget    | publicPath         | serverBuildPath                         | serverConditions | serverMainFields        | serverModuleFormat | serverPlatform | serverDependenciesToBundle | serverMinify |
+| -------------------- | ------------------ | --------------------------------------- | ---------------- | ----------------------- | ------------------ | -------------- | -------------------------- | ------------ |
+| `arc`                | `/\_static/build/` | `server/index.js`                       |                  | `main, module`          | `cjs`              | `node`         |                            | `false`      |
+| `cloudflare-pages`   | `/build/`          | `functions/[[path]].js`                 | `worker`         | `browser, module, main` | `esm`              | `neutral`      | `all`                      | `true`       |
+| `cloudflare-workers` | `/build/`          | `build/index.js`                        | `worker`         | `browser, module, main` | `esm`              | `neutral`      | `all`                      | `true`       |
+| `deno`               | `/build/`          | `build/index.js`                        | `deno, worker`   | `module, main`          | `esm`              | `neutral`      | `all`                      | `false`      |
+| `netlify`            | `/build/`          | `.netlify/functions-internal/server.js` |                  | `main, module`          | `cjs`              | `node`         |                            | `false`      |
+| `node-cjs`           | `/build/`          | `build/index.js`                        |                  | `main, module`          | `cjs`              | `node`         |                            | `false`      |
+| `vercel`             | `/build/`          | `api/index.js`                          |                  | `main, module`          | `cjs`              | `node`         |                            | `false`      |
+
 ## serverConditions
 
 The order of conditions to use when resolving server dependencies' `exports`
diff --git a/integration/tsconfig.json b/integration/tsconfig.json
index b8fd6c97a..d22159441 100644
--- a/integration/tsconfig.json
+++ b/integration/tsconfig.json
@@ -17,6 +17,7 @@
     "rootDir": "."
   },
   "references": [
+    { "path": "../packages/remix-dev" },
     { "path": "../packages/remix-express" },
     { "path": "../packages/remix-react" },
     { "path": "../packages/remix-server-runtime" }
diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts
index a511ad616..9a1f54386 100644
--- a/packages/remix-dev/config.ts
+++ b/packages/remix-dev/config.ts
@@ -401,6 +401,10 @@ export async function readConfig(
     }
   }
 
+  if (appConfig.serverBuildTarget) {
+    warnOnce(serverBuildTargetWarning, "v2_serverBuildTarget");
+  }
+  
   if (!appConfig.future?.v2_errorBoundary) {
     warnOnce(errorBoundaryWarning, "v2_errorBoundary");
   }
@@ -564,9 +568,14 @@ export async function readConfig(
     root: { path: "", id: "root", file: rootRouteFile },
   };
 
-  let routesConvention = appConfig.future?.v2_routeConvention
-    ? flatRoutes
-    : defineConventionalRoutes;
+  let routesConvention: typeof flatRoutes;
+
+  if (appConfig.future?.v2_routeConvention) {
+    routesConvention = flatRoutes;
+  } else {
+    warnOnce(flatRoutesWarning, "v2_routeConvention");
+    routesConvention = defineConventionalRoutes;
+  }
 
   if (fse.existsSync(path.resolve(appDirectory, "routes"))) {
     let conventionalRoutes = routesConvention(
@@ -730,10 +739,15 @@ let listFormat = new Intl.ListFormat("en", {
   type: "conjunction",
 });
 
+
+export let serverBuildTargetWarning = `⚠️ DEPRECATED: The "serverBuildTarget" config option is deprecated. Use a combination of "publicPath", "serverBuildPath", "serverConditions", "serverDependenciesToBundle", "serverMainFields", "serverMinify", "serverModuleFormat" and/or "serverPlatform" instead.`;
+
+export let flatRoutesWarning = `⚠️ DEPRECATED: The old nested folders route convention has been deprecated in favor of "flat routes".  Please enable the new routing convention via the \`future.v2_routeConvention\` flag in your \`remix.config.js\` file.  For more information, please see https://remix.run/docs/en/main/file-conventions/route-files-v2.`;
+
 export const errorBoundaryWarning =
   "⚠️ DEPRECATED: The separation of `CatchBoundary` and `ErrorBoundary` has " +
   "been deprecated and Remix v2 will use a singular `ErrorBoundary` for " +
   "all thrown values (`Response` and `Error`). Please migrate to the new " +
   "behavior in Remix v1 via the `future.v2_errorBoundary` flag in your " +
   "`remix.config.js` file. For more information, see " +
-  "https://remix.run/docs/route/error-boundary-v2";
+  "https://remix.run/docs/route/error-boundary-v2";
\ No newline at end of file


Test patch:
diff --git a/integration/flat-routes-test.ts b/integration/flat-routes-test.ts
index 9254ca16e..0b4b6b6aa 100644
--- a/integration/flat-routes-test.ts
+++ b/integration/flat-routes-test.ts
@@ -5,6 +5,7 @@ import { PlaywrightFixture } from "./helpers/playwright-fixture";
 import type { Fixture, AppFixture } from "./helpers/create-fixture";
 import { createFixtureProject } from "./helpers/create-fixture";
 import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
+import { flatRoutesWarning } from "../packages/remix-dev/config";
 
 let fixture: Fixture;
 let appFixture: AppFixture;
@@ -147,6 +148,52 @@ test.describe("flat routes", () => {
   }
 });
 
+test.describe("warns when v1 routesConvention is used", () => {
+  let buildStdio = new PassThrough();
+  let buildOutput: string;
+
+  let originalConsoleLog = console.log;
+  let originalConsoleWarn = console.warn;
+  let originalConsoleError = console.error;
+
+  test.beforeAll(async () => {
+    console.log = () => {};
+    console.warn = () => {};
+    console.error = () => {};
+    await createFixtureProject({
+      buildStdio,
+      future: { v2_routeConvention: false },
+      files: {
+        "routes/index.tsx": js`
+          export default function () {
+            return <p>routes/index</p>;
+          }
+        `,
+      },
+    });
+
+    let chunks: Buffer[] = [];
+    buildOutput = await new Promise<string>((resolve, reject) => {
+      buildStdio.on("data", (chunk) => chunks.push(Buffer.from(chunk)));
+      buildStdio.on("error", (err) => reject(err));
+      buildStdio.on("end", () =>
+        resolve(Buffer.concat(chunks).toString("utf8"))
+      );
+    });
+  });
+
+  test.afterAll(() => {
+    console.log = originalConsoleLog;
+    console.warn = originalConsoleWarn;
+    console.error = originalConsoleError;
+  });
+
+  test("warns about conflicting routes", () => {
+    console.log(buildOutput);
+    expect(buildOutput).toContain(flatRoutesWarning);
+  });
+});
+
 test.describe("emits warnings for route conflicts", async () => {
   let buildStdio = new PassThrough();
   let buildOutput: string;
diff --git a/packages/remix-dev/__tests__/create-test.ts b/packages/remix-dev/__tests__/create-test.ts
index d0db46377..2f9eb196b 100644
--- a/packages/remix-dev/__tests__/create-test.ts
+++ b/packages/remix-dev/__tests__/create-test.ts
@@ -8,7 +8,7 @@ import stripAnsi from "strip-ansi";
 
 import { run } from "../cli/run";
 import { server } from "./msw";
-import { errorBoundaryWarning } from "../config";
+import { errorBoundaryWarning, flatRoutesWarning } from "../config";
 
 beforeAll(() => server.listen({ onUnhandledRequest: "error" }));
 afterAll(() => server.close());
@@ -349,6 +349,8 @@ describe("the create command", () => {
     ]);
     expect(output.trim()).toBe(
       errorBoundaryWarning +
+        "\n" +
+        flatRoutesWarning +
         "\n\n" +
         getOptOutOfInstallMessage() +
         "\n\n" +
diff --git a/packages/remix-dev/__tests__/fixtures/stack/remix.config.js b/packages/remix-dev/__tests__/fixtures/stack/remix.config.js
index adf2a0b5d..5bb0822e7 100644
--- a/packages/remix-dev/__tests__/fixtures/stack/remix.config.js
+++ b/packages/remix-dev/__tests__/fixtures/stack/remix.config.js
@@ -1,5 +1,6 @@
 /** @type {import('@remix-run/dev').AppConfig} */
 module.exports = {
+  serverBuildTarget: "node-cjs",
   ignoredRouteFiles: ["**/.*"],
   // appDirectory: "app",
   // assetsBuildDirectory: "public/build",
diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts
index 1dced697b..b953178b3 100644
--- a/packages/remix-dev/__tests__/readConfig-test.ts
+++ b/packages/remix-dev/__tests__/readConfig-test.ts
@@ -1,17 +1,23 @@
 import path from "path";
 
 import type { RemixConfig } from "../config";
-import { readConfig } from "../config";
+import { serverBuildTargetWarning, readConfig } from "../config";
 
 const remixRoot = path.resolve(__dirname, "./fixtures/stack");
 
 describe("readConfig", () => {
   let config: RemixConfig;
+  let warnStub;
   beforeEach(async () => {
+    let consoleWarn = console.warn;
+    warnStub = jest.fn();
+    console.warn = warnStub;
     config = await readConfig(remixRoot);
+    console.warn = consoleWarn;
   });
 
   it("generates a config", async () => {
+    expect(warnStub).toHaveBeenCalledWith(serverBuildTargetWarning);
     expect(config).toMatchInlineSnapshot(
       {
         rootDirectory: expect.any(String),
@@ -69,7 +75,7 @@ describe("readConfig", () => {
           },
         },
         "serverBuildPath": Any<String>,
-        "serverBuildTarget": undefined,
+        "serverBuildTarget": "node-cjs",
         "serverBuildTargetEntryModule": "export * from \\"@remix-run/dev/server-build\\";",
         "serverConditions": undefined,
         "serverDependenciesToBundle": Array [],