-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Conversation
🦋 Changeset detectedLatest commit: 821f914 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -66,7 +66,7 @@ export default function handleRequest( | |||
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 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 [email protected]
return isbot(request.headers.get("user-agent") || "") | |
return isbot(request.headers.get("user-agent")) |
@@ -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 comment
The 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
@@ -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 comment
The reason will be displayed to describe this comment to others. Learn more.
going forward, remix dev
will add v4 to the package.json
instead of latest
- this is the primary "fix"
@@ -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 comment
The reason will be displayed to describe this comment to others. Learn more.
Internal implementations of entry.server.tsx
will support either v3 or v4 for backward compatibility with existing Remix apps who have pinned v3 in their package.json
@@ -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 comment
The 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 comment
The 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
Accept null in place of user agent string to allow header value to be used "as is" (request.headers.get("user-agent"))
Co-authored-by: Filip Bel <[email protected]>
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Background:
isbot
installed for use by the default providedentry.server.tsx
(and the one internally used byremix dev
if one is not present in the app)remix dev
addsisbot
topackage.json
if it does not exist but it does so with thelatest
tagisbot
released v4 on 1/1/2024 and moved from a default to a named exportlatest
v4 version ran into build/runtime issues since theirentry.server
(or the internal one) assumed a default export which no longer existedlatest
- it should not be an issue AFAIK for apps using lockfiles since those should have the v3 version pinned internallyisbot
fromlatest
to^3
in theirpackage.json
Solution
isbot
v4 andremix dev
will update package.json with"isbot": "^4"
instead oflatest
remix dev
can't assume v4 since there are Remix apps in the wild that may have"isbot": "3.x.y"
in theirpackage.json
(if they changed it fromlatest
) that are using the internal entry.server.tsx file.remix dev
defaults to handle bothisbot
v3 or v4entry.server.tsx
in templates will assume v4 for net new appsReimplements #8385 which was reverted in #8414
Closes #8407, #8392