Skip to content

fix: #526 CLI warn if using @trigger.dev/react package with Next.js pages directory #530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
neo773 wants to merge 3 commits into from
Closed

fix: #526 CLI warn if using @trigger.dev/react package with Next.js pages directory #530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12b06bf
fix: #526
neo773 Oct 2, 2023
d8e4356
add check for @trigger.dev/react
neo773 Oct 2, 2023
e96dbe5
Merge branch 'main' into 526
matt-aitken Oct 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"@types/mock-fs": "^4.13.1",
"@types/node": "16",
"@types/node-fetch": "^2.6.2",
"@types/semver": "^7.3.13",
"rimraf": "^3.0.2",
"tsup": "^6.5.0",
"type-fest": "^3.6.0",
Expand Down Expand Up @@ -80,6 +81,7 @@
"path-to-regexp": "^6.2.1",
"posthog-node": "^3.1.1",
"proxy-agent": "^6.3.0",
"semver": "^7.5.0",
"simple-git": "^3.19.0",
"terminal-link": "^3.0.0",
"tsconfck": "^2.1.2",
Expand Down
55 changes: 55 additions & 0 deletions packages/cli/src/frameworks/nextjs/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import { readPackageJson } from "../../utils/readPackageJson";
import { standardWatchFilePaths } from "../watchConfig";
import { telemetryClient } from "../../telemetry/telemetry";
import { detectMiddlewareUsage } from "./middleware";
import semver from "semver";
import boxen from "boxen";
import { createRequire } from "node:module";

export class NextJs implements Framework {
id = "nextjs";
Expand Down Expand Up @@ -89,6 +92,58 @@ export class NextJs implements Framework {
break;
}
}

const nextJsDir = await detectPagesOrAppDir(path);
Copy link
Member

@matt-aitken matt-aitken Oct 13, 2023

Choose a reason for hiding this comment

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

Let's create a separate function for all of this logic, and export it from this file so tests can be written.

Could you also wrap the function in a try catch block so if an error is thrown it doesn't break installs. It doesn't need to do anything in the catch.

const nextConfigExists = await detectNextConfigFile(path);

const require = createRequire(import.meta.url);
const nextPath = require.resolve("next");
const reactPath = require.resolve("@trigger.dev/react");

if (nextConfigExists && nextJsDir === "pages" && typeof reactPath !== "undefined") {
const nextPackageJsonPath = pathModule.join(nextPath, "../../../", "package.json");
const nextPackageJsonData = JSON.parse(await fs.readFile(nextPackageJsonPath, "utf8"));
Comment on lines +96 to +105
Copy link
Member

@matt-aitken matt-aitken Oct 13, 2023

Choose a reason for hiding this comment

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

It would be much better to use the current way we check for dependencies.

This function gets the "next" dependency version, and then returns true if it exists. You could create a function that uses this same methodology where you can pass in a package name and it returns undefined or the version string.

trigger.dev/packages/cli/src/frameworks/nextjs/index.ts

Line 111 in 6769d6b

export async function detectNextDependency(path: string): Promise<boolean> {


const nextVersion = nextPackageJsonData.version;

if (semver.gte(nextVersion, "13.1.0")) {
logger.warn(
`⚠️ We've detected you're using Next.js Pages, please add the following to your next.config`
);
logger.info(
boxen(
`
const nextConfig = {
// existing code
transpilePackages: ["@trigger.dev/react"],
};
module.exports = nextConfig;
`,
{ padding: 1, borderStyle: "double", borderColor: "magenta" }
)
);
}

if (semver.lt(nextVersion, "13.1.0")) {
logger.warn(
`⚠️ We've detected you're using Next.js Pages, please add the following to your next.config`
);
logger.info(
boxen(
`
Add the following to your next.config file:

const withTM = require('next-transpile-modules')(['@trigger.dev/react']);

module.exports = withTM({
//...existing config object
});
`,
{ margin: 1, borderStyle: "double", borderColor: "magenta" }
)
);
}
}
}

defaultHostnames = ["localhost"];
Expand Down