-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle isbot v4 in a backwards-compatible manner #8415
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@remix-run/dev": patch | ||
--- | ||
|
||
Update built-in `entry.*` templates to work with `isbot@3` or `isbot@4` while updating templates to use `isbot@4` moving forward |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -172,6 +172,7 @@ | |
- fergusmeiklejohn | ||
- fernandojbf | ||
- fgiuliani | ||
- fifi98 | ||
- fishel-feng | ||
- francisudeji | ||
- frandiox | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -985,7 +985,7 @@ test.describe("aborted", () => { | |
import type { AppLoadContext, EntryContext } from "@remix-run/node"; | ||
import { createReadableStreamFromReadable } from "@remix-run/node"; | ||
import { RemixServer } from "@remix-run/react"; | ||
import isbot from "isbot"; | ||
import { isbot } from "isbot"; | ||
import { renderToPipeableStream } from "react-dom/server"; | ||
|
||
const ABORT_DELAY = 1; | ||
|
@@ -997,7 +997,7 @@ test.describe("aborted", () => { | |
remixContext: EntryContext, | ||
loadContext: AppLoadContext | ||
) { | ||
return isbot(request.headers.get("user-agent")) | ||
return isbot(request.headers.get("user-agent") || "") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All internal unit/e2e tests and the repo itself are updated to use v4 |
||
? handleBotRequest( | ||
request, | ||
responseStatusCode, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -493,7 +493,7 @@ export async function resolveConfig( | |
pkgJson.update({ | ||
dependencies: { | ||
...pkgJson.content.dependencies, | ||
isbot: "latest", | ||
isbot: "^4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. going forward, |
||
}, | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import type { AppLoadContext, EntryContext } from "@remix-run/cloudflare"; | ||
import { RemixServer } from "@remix-run/react"; | ||
import isbot from "isbot"; | ||
import isbotModule from "isbot"; | ||
import { renderToReadableStream } from "react-dom/server"; | ||
|
||
export default async function handleRequest( | ||
|
@@ -22,7 +22,7 @@ export default async function handleRequest( | |
} | ||
); | ||
|
||
if (isbot(request.headers.get("user-agent"))) { | ||
if (isBotRequest(request.headers.get("user-agent"))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Internal implementations of |
||
await body.allReady; | ||
} | ||
|
||
|
@@ -32,3 +32,24 @@ export default async function handleRequest( | |
status: responseStatusCode, | ||
}); | ||
} | ||
|
||
// We have some Remix apps in the wild already running with isbot@3 so we need | ||
// to maintain backwards compatibility even though we want new apps to use | ||
// isbot@4. That way, we can ship this as a minor Semver update to @remix-run/dev. | ||
function isBotRequest(userAgent: string | null) { | ||
if (!userAgent) { | ||
return false; | ||
} | ||
|
||
// isbot >= 3.8.0, >4 | ||
if ("isbot" in isbotModule && typeof isbotModule.isbot === "function") { | ||
return isbotModule.isbot(userAgent); | ||
} | ||
|
||
// isbot < 3.8.0 | ||
if ("default" in isbotModule && typeof isbotModule.default === "function") { | ||
return isbotModule.default(userAgent); | ||
} | ||
|
||
return false; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ import { PassThrough } from "node:stream"; | |
import type { AppLoadContext, EntryContext } from "@remix-run/node"; | ||
import { createReadableStreamFromReadable } from "@remix-run/node"; | ||
import { RemixServer } from "@remix-run/react"; | ||
import isbot from "isbot"; | ||
import { isbot } from "isbot"; | ||
import { renderToPipeableStream } from "react-dom/server"; | ||
|
||
const ABORT_DELAY = 5_000; | ||
|
@@ -24,7 +24,7 @@ export default function handleRequest( | |
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
loadContext: AppLoadContext | ||
) { | ||
return isbot(request.headers.get("user-agent")) | ||
return isbot(request.headers.get("user-agent") || "") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Templates use v4 API moving forward There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The use case has been addressed with version 4.2.0 to accept "null" as input, with the intention to keep user code cleaner. https://github.com/omrilotan/isbot/blob/main/CHANGELOG.md#420
|
||
? handleBotRequest( | ||
request, | ||
responseStatusCode, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All docs are updated to reflect the v4 API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This use case is supported as is starting isbot@4.2.0