Skip to content
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

Update realtime docs for streaming and a couple tweaks to the SDK #1486

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

ericallam
Copy link
Member

@ericallam ericallam commented Nov 20, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • React hooks now accept accessToken and baseURL options directly, simplifying authentication and API integration.
    • Introduced new hooks, including useRealtimeRunWithStreams, for enhanced data streaming capabilities.
    • Added a Realtime API for monitoring tasks in real-time.
  • Documentation

    • Updated documentation for React hooks, emphasizing new authentication options and usage examples.
    • Expanded sections on Public Access Tokens and metadata methods for better clarity and usability.
    • Introduced new documentation files for Realtime streams and React hooks, providing comprehensive guides for developers.

Copy link

changeset-bot bot commented Nov 20, 2024

🦋 Changeset detected

Latest commit: 8b68a82

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@trigger.dev/react-hooks Patch
@trigger.dev/sdk Patch
@trigger.dev/build Patch
@trigger.dev/core Patch
@trigger.dev/rsc Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch
trigger.dev Patch
@internal/redis-worker Patch
@internal/zod-worker Patch
@internal/testcontainers Patch

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

Copy link
Contributor

coderabbitai bot commented Nov 20, 2024

Walkthrough

The changes in this pull request primarily focus on enhancing the functionality of React hooks and related documentation within the Trigger.dev SDK. The modifications allow hooks to directly accept accessToken and baseURL options, eliminating the need for a Provider. Additionally, the documentation has been updated to reflect these changes, emphasizing the use of hooks for authentication and task management. New hooks and methods have been introduced, while some outdated documentation has been removed to streamline the user experience.

Changes

File Path Change Summary
.changeset/khaki-ants-yawn.md Updated React hooks to accept accessToken and baseURL options directly.
docs/frontend/overview.mdx Revised documentation on authentication and Public Access Tokens, emphasizing React hooks usage and clarifying token creation.
docs/frontend/react-hooks.mdx Updated React hooks documentation to include optional accessToken parameter, modified examples, and introduced new hooks like useRealtimeRunWithStreams.
docs/introduction.mdx Added a new feature section for Realtime API and React hooks, reformatted YouTube video embedding.
docs/mint.json Reformatted JSON arrays for improved readability without changing functionality.
docs/realtime/overview.mdx Added a new section titled "Realtime streams" for better documentation on the Realtime API.
docs/realtime/react-hooks.mdx Created a new documentation file for "Realtime React hooks" with a description of its functionality.
docs/realtime/streams.mdx Introduced documentation for the Streams API, detailing how to register and subscribe to streams, including examples with OpenAI SDK integration.
docs/realtime/use-realtime-run.mdx Removed documentation for the useRealtimeRun hook.
docs/realtime/use-realtime-runs-with-tag.mdx Removed documentation for the useRealtimeRunsWithTag hook.
docs/runs/metadata.mdx Updated metadata documentation with new methods for accessing and manipulating metadata, emphasizing type safety and synchronous behavior of update methods.
packages/react-hooks/src/hooks/useRun.ts Modified useRun hook to include options parameter for API client configuration and updated refresh interval logic.
packages/react-hooks/src/utils/trigger-swr.ts Enhanced CommonTriggerHookOptions type with new optional properties for better data fetching and revalidation control.
packages/trigger-sdk/package.json Updated ai dependency version and enhanced exports structure for improved module access.
references/nextjs-realtime/src/trigger/ai.ts Created new OpenAI SDK instance, modified streaming tasks, and introduced new schema tasks for fetching streams and OpenAI SDK streaming.

Suggested reviewers

  • matt-aitken

🐰 In the meadow, hops a little sprite,
With hooks that now feel just right.
Access tokens, base URLs in tow,
Streamlined paths for all to know.
React's magic, a joyous delight,
Coding's a dance, under moonlight! 🌙✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/react-hooks/src/hooks/useRun.ts

(node:30026) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files
(Use node --trace-warnings ... to show where the warning was created)

Oops! Something went wrong! :(

ESLint: 9.15.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ericallam ericallam merged commit af7a768 into main Nov 20, 2024
8 of 9 checks passed
@ericallam ericallam deleted the docs/realtime-streams branch November 20, 2024 14:10
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (20)
.changeset/khaki-ants-yawn.md (1)

6-7: Consider enhancing the changeset description

While accurate, the description could be more comprehensive. Consider adding:

  • A comma before 'so' for better readability
  • Migration guidance for users currently using the Provider
  • Examples of the new options usage
-React hooks now all accept accessToken and baseURL options so the use of the Provider is no longer necessary
+React hooks now all accept accessToken and baseURL options, so the use of the Provider is no longer necessary. Users can now directly pass these options to individual hooks:
+
+```jsx
+const run = useRealtimeRun(id, { accessToken: "your-token", baseURL: "your-url" });
+```
+
+If you're currently using the Provider, you can gradually migrate to the new approach while maintaining backward compatibility.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~6-~6: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...l accept accessToken and baseURL options so the use of the Provider is no longer ne...

(COMMA_COMPOUND_SENTENCE_2)

packages/react-hooks/src/utils/trigger-swr.ts (2)

11-25: Add JSDoc comments for authentication and API-related options

The type changes look good and align with the goal of eliminating Provider dependency. Consider adding JSDoc comments for the authentication and API-related options to maintain consistency with the existing documentation style.

  /** Revalidate the data when the window regains focus. */
  revalidateOnFocus?: boolean;

-  /** Optional access token for authentication */
+  /** 
+   * Access token for authentication. When provided, this will override any token 
+   * configured through the Provider.
+   */
  accessToken?: string;
-  /** Optional base URL for the API endpoints */
+  /**
+   * Base URL for the API endpoints. When provided, this will override any URL
+   * configured through the Provider.
+   */
  baseURL?: string;
-  /** Optional additional request configuration */
+  /**
+   * Additional request configuration options that will be passed to all API calls
+   * made by this hook.
+   */
  requestOptions?: ApiRequestOptions;

11-14: Consider real-time update strategy

The refreshInterval polling option is correctly documented as not recommended in favor of Realtime hooks. However, consider adding a reference to the specific Realtime hooks that should be used instead, making it easier for developers to follow the recommended approach.

packages/react-hooks/src/hooks/useRun.ts (1)

Line range hint 7-19: Enhance JSDoc documentation for better developer experience

Consider improving the documentation by:

  1. Adding details about available options (refreshInterval, revalidateOnReconnect, revalidateOnFocus, etc.)
  2. Using TypeScript return type syntax instead of separate @returns for each property
 /**
  * Custom hook to retrieve and manage the state of a run by its ID.
  *
  * @template TTask - The type of the task associated with the run.
  * @param {string} runId - The unique identifier of the run to retrieve.
- * @param {CommonTriggerHookOptions} [options] - Optional configuration for the hook's behavior.
- * @returns {Object} An object containing the run data, error, loading state, validation state, and error state.
- * @returns {RetrieveRunResult<TTask> | undefined} run - The retrieved run data.
- * @returns {Error | undefined} error - The error object if an error occurred.
- * @returns {boolean} isLoading - Indicates if the run data is currently being loaded.
- * @returns {boolean} isValidating - Indicates if the run data is currently being validated.
- * @returns {boolean} isError - Indicates if an error occurred during the retrieval of the run data.
+ * @param {CommonTriggerHookOptions} [options] - Optional configuration:
+ *   - refreshInterval?: number - Interval in milliseconds for data refresh
+ *   - revalidateOnReconnect?: boolean - Whether to revalidate when reconnecting
+ *   - revalidateOnFocus?: boolean - Whether to revalidate when window regains focus
+ *   - accessToken?: string - API access token
+ *   - baseURL?: string - API base URL
+ * @returns {{
+ *   run: RetrieveRunResult<TTask> | undefined,
+ *   error: Error | undefined,
+ *   isLoading: boolean,
+ *   isValidating: boolean,
+ *   isError: boolean
+ * }}
  */
docs/introduction.mdx (1)

15-15: LGTM, with a style suggestion.

The new bullet point effectively introduces the Realtime API feature and includes proper documentation links.

Consider rephrasing to avoid repetitive sentence beginnings. Here's a suggestion:

-- We provide a [Realtime API](/realtime) for monitoring tasks in real-time, along with [React hooks](/frontend/react-hooks#realtime-hooks) for building custom dashboards.
++ Monitor tasks in real-time using our [Realtime API](/realtime), complete with [React hooks](/frontend/react-hooks#realtime-hooks) for building custom dashboards.
🧰 Tools
🪛 LanguageTool

[style] ~15-~15: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., debugging, and managing your tasks. - We provide a Realtime API for...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

docs/frontend/overview.mdx (3)

11-11: Consider adding security best practices.

While the documentation clearly explains how to create tokens, it would be beneficial to add a note about security best practices, such as:

  • Proper storage and transmission of tokens
  • Token rotation strategies
  • Environment-specific considerations

94-94: Fix grammatical error in the sentence.

Change "which is required" to "which are required" to maintain proper subject-verb agreement.

-You can also specify write scopes, which is required for triggering tasks from your frontend application:
+You can also specify write scopes, which are required for triggering tasks from your frontend application:
🧰 Tools
🪛 LanguageTool

[uncategorized] ~94-~94: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct.
Context: ...ou can also specify write scopes, which is required for triggering tasks from your...

(AI_HYDRA_LEO_CPT_IS_ARE)


167-169: Consider adding a quick example.

While linking to the React hooks documentation is good, consider adding a brief inline example to give readers a quick start, such as how to use a token with a basic hook.

docs/runs/metadata.mdx (5)

28-34: Clarify the difference between metadata.get() and metadata.current()

While both methods are shown, it would be helpful to explain the distinction between metadata.get() and metadata.current(). Consider adding a note explaining when to use each method.


112-112: Fix typo in metadata update methods description

-All metadata update methods (accept for `flush` and `stream`) are synchronous
+All metadata update methods (except for `flush` and `stream`) are synchronous
🧰 Tools
🪛 LanguageTool

[misspelling] ~112-~112: Did you mean “except”?
Context: ...ur runs.) All metadata update methods (accept for flush and stream) are synchrono...

(ACCEPT_EXCEPT)


338-357: Enhance OpenAI streaming example with error handling

The OpenAI streaming example could be improved by adding error handling and showing how to handle stream interruptions. Consider adding:

  1. Try-catch block around the API call
  2. Error handling for stream interruptions
  3. Cleanup in case of errors

Example enhancement:

 export const myTask = task({
   id: "my-task",
   run: async (payload: { prompt: string }) => {
+    try {
       const completion = await openai.chat.completions.create({
         messages: [{ role: "user", content: payload.prompt }],
         model: "gpt-3.5-turbo",
         stream: true,
       });
 
       const stream = await metadata.stream("openai", completion);
 
       let text = "";
 
       for await (const chunk of stream) {
         logger.log("Received chunk", { chunk });
         text += chunk.choices.map((choice) => choice.delta?.content).join("");
       }
 
       return { text };
+    } catch (error) {
+      logger.error("Error in OpenAI streaming", { error });
+      throw error;
+    }
   },
 });

Line range hint 441-465: Add performance consideration note for Zod validation

The Zod validation example is good, but consider adding a note about performance implications:

  1. Mention that validation adds runtime overhead
  2. Suggest using .safeParse() instead of .parse() in production
  3. Consider recommending validation only in development or at specific checkpoints
🧰 Tools
🪛 LanguageTool

[style] ~56-~56: This phrase is redundant. Consider using “outside”.
Context: ...If you call any of the metadata methods outside of the run function, they will have no eff...

(OUTSIDE_OF)


[misspelling] ~112-~112: Did you mean “except”?
Context: ...ur runs.) All metadata update methods (accept for flush and stream) are synchrono...

(ACCEPT_EXCEPT)


Line range hint 508-508: Add best practices for handling large metadata

Consider adding a section about best practices for handling large metadata:

  1. How to structure metadata to stay within limits
  2. Strategies for splitting large data across multiple keys
  3. When to use external storage instead of metadata
🧰 Tools
🪛 LanguageTool

[style] ~56-~56: This phrase is redundant. Consider using “outside”.
Context: ...If you call any of the metadata methods outside of the run function, they will have no eff...

(OUTSIDE_OF)


[misspelling] ~112-~112: Did you mean “except”?
Context: ...ur runs.) All metadata update methods (accept for flush and stream) are synchrono...

(ACCEPT_EXCEPT)

docs/realtime/streams.mdx (5)

25-27: Consider consolidating STREAMS type definitions

The STREAMS type is defined twice with different contents. Consider consolidating these definitions into a single, comprehensive type definition at the beginning of the documentation to avoid confusion.

export type STREAMS = {
  openai: OpenAI.ChatCompletionChunk;
+ fetch: string;
};

Also applies to: 97-100


127-132: Consider highlighting stream timeout information

The note about stream consumption timeout is crucial information that could affect production deployments. Consider moving this information to a more prominent position, perhaps in the "How it works" section, and formatting it as a warning rather than a note.


173-195: Consider enhancing the React example with error handling and better loading states

The current example could be improved to better demonstrate production-ready code:

  1. Add error handling for failed runs
  2. Implement a proper loading state component
  3. Add type safety for the streams object
function MyComponent({ runId, publicAccessToken }: { runId: string; publicAccessToken: string }) {
-  const { run, streams } = useRealtimeRunWithStreams<typeof myTask, STREAMS>(runId, {
+  const { run, streams, error, isLoading } = useRealtimeRunWithStreams<typeof myTask, STREAMS>(runId, {
    accessToken: publicAccessToken,
  });

+  if (error) {
+    return <div>Error: {error.message}</div>;
+  }

-  if (!run) {
+  if (isLoading || !run) {
-    return <div>Loading...</div>;
+    return <LoadingSpinner />;
  }

  return (
    <div>
      <h1>Run ID: {run.id}</h1>
      <h2>Streams:</h2>
      <ul>
-        {Object.entries(streams).map(([key, value]) => (
+        {Object.entries(streams ?? {}).map(([key, value]) => (
          <li key={key}>
            <strong>{key}</strong>: {JSON.stringify(value)}
          </li>
        ))}
      </ul>
    </div>
  );
}

398-404: Add comment indicating mock implementation

The weather data implementation uses mock data. Consider adding a comment to make it clear this is for demonstration purposes only.

  run: async ({ location }) => {
+   // NOTE: This is a mock implementation for demonstration purposes
+   // Replace with actual weather API integration in production
    return {
      location,
      temperature: 72 + Math.floor(Math.random() * 21) - 10,
    };
  },

420-420: Fix typo in default prompt

There's a typo in "San Fransico" (should be "San Francisco").

- "Based on the temperature, will I need to wear extra clothes today in San Fransico? Please be detailed."
+ "Based on the temperature, will I need to wear extra clothes today in San Francisco? Please be detailed."
docs/frontend/react-hooks.mdx (1)

448-449: Fix grammar in the sentence.

Change "the type of the streams" to "the types of the streams" to maintain proper agreement with the plural noun.

🧰 Tools
🪛 LanguageTool

[grammar] ~448-~448: In this context, ‘type’ should agree in number with the noun after ‘of’.
Context: ... ); } ``` You can provide the type of the streams to the useRealtimeRunWithStreams hook...

(TYPE_OF_PLURAL)

references/nextjs-realtime/src/trigger/ai.ts (1)

205-209: Handle possible undefined content when aggregating text

There is a potential for choice.delta?.content to be undefined, which could result in undefined being concatenated to text. To prevent this, filter out undefined values before joining.

Apply this diff:

           text += chunk.choices
             .map((choice) => choice.delta?.content)
+            .filter((content): content is string => content !== undefined)
             .join("");
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c044cb1 and 8b68a82.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (15)
  • .changeset/khaki-ants-yawn.md (1 hunks)
  • docs/frontend/overview.mdx (3 hunks)
  • docs/frontend/react-hooks.mdx (8 hunks)
  • docs/introduction.mdx (1 hunks)
  • docs/mint.json (9 hunks)
  • docs/realtime/overview.mdx (1 hunks)
  • docs/realtime/react-hooks.mdx (1 hunks)
  • docs/realtime/streams.mdx (1 hunks)
  • docs/realtime/use-realtime-run.mdx (0 hunks)
  • docs/realtime/use-realtime-runs-with-tag.mdx (0 hunks)
  • docs/runs/metadata.mdx (2 hunks)
  • packages/react-hooks/src/hooks/useRun.ts (1 hunks)
  • packages/react-hooks/src/utils/trigger-swr.ts (1 hunks)
  • packages/trigger-sdk/package.json (2 hunks)
  • references/nextjs-realtime/src/trigger/ai.ts (3 hunks)
💤 Files with no reviewable changes (2)
  • docs/realtime/use-realtime-run.mdx
  • docs/realtime/use-realtime-runs-with-tag.mdx
✅ Files skipped from review due to trivial changes (3)
  • docs/mint.json
  • docs/realtime/overview.mdx
  • docs/realtime/react-hooks.mdx
🧰 Additional context used
🪛 LanguageTool
.changeset/khaki-ants-yawn.md

[uncategorized] ~6-~6: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...l accept accessToken and baseURL options so the use of the Provider is no longer ne...

(COMMA_COMPOUND_SENTENCE_2)

docs/frontend/overview.mdx

[uncategorized] ~94-~94: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct.
Context: ...ou can also specify write scopes, which is required for triggering tasks from your...

(AI_HYDRA_LEO_CPT_IS_ARE)

docs/frontend/react-hooks.mdx

[grammar] ~448-~448: In this context, ‘type’ should agree in number with the noun after ‘of’.
Context: ... ); } ``` You can provide the type of the streams to the useRealtimeRunWithStreams hook...

(TYPE_OF_PLURAL)

docs/introduction.mdx

[style] ~15-~15: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., debugging, and managing your tasks. - We provide a Realtime API for...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...hooks) for building custom dashboards. We're [open source](https://github.com/tri...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

docs/realtime/streams.mdx

[style] ~7-~7: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...e world in realtime. This is useful for a variety of use cases, including AI. ## How it wor...

(A_VARIETY_OF)

docs/runs/metadata.mdx

[style] ~56-~56: This phrase is redundant. Consider using “outside”.
Context: ...If you call any of the metadata methods outside of the run function, they will have no eff...

(OUTSIDE_OF)


[misspelling] ~112-~112: Did you mean “except”?
Context: ...ur runs.) All metadata update methods (accept for flush and stream) are synchrono...

(ACCEPT_EXCEPT)


[uncategorized] ~294-~294: Possible missing comma found.
Context: ...and made available in the Realtime API. So for example, you could pass the body of...

(AI_HYDRA_LEO_MISSING_COMMA)

🔇 Additional comments (11)
.changeset/khaki-ants-yawn.md (1)

1-4: LGTM on version bumps!

The patch version increments are appropriate since these changes add functionality in a backward-compatible way.

packages/react-hooks/src/utils/trigger-swr.ts (1)

2-4: LGTM: Import changes are appropriate

The import of ApiRequestOptions from the v3 core package is correctly added to support the new request configuration options.

packages/react-hooks/src/hooks/useRun.ts (1)

Line range hint 30-46: LGTM! Well-implemented hook with smart refresh handling

The implementation shows good practices:

  • Efficient use of SWR for data fetching and caching
  • Smart refresh interval handling that stops polling when the run is completed
  • Proper propagation of configuration options
  • Solid error handling
packages/trigger-sdk/package.json (1)

69-69: Verify the major version upgrade of 'ai' package

The 'ai' package has been upgraded from ^3.4.33 to ^4.0.1, which is a major version bump. This could introduce breaking changes that need to be addressed.

Please ensure:

  1. The upgrade is intentional and aligned with the PR objectives
  2. All breaking changes from [email protected] have been accounted for
  3. The application has been tested with this new version
docs/introduction.mdx (1)

22-31: LGTM! Well-formatted iframe with proper security attributes.

The reformatted iframe improves readability while maintaining all necessary security attributes and YouTube embed functionality.

docs/frontend/overview.mdx (1)

7-7: LGTM! Clear introduction with proper documentation linking.

docs/realtime/streams.mdx (1)

343-344: ⚠️ Potential issue

Remove duplicate import statement

The useRealtimeRunWithStreams import is duplicated.

- import { useRealtimeRunWithStreams } from "@trigger.dev/sdk/v3";
import { useRealtimeRunWithStreams } from "@trigger.dev/sdk/v3";

Likely invalid or redundant comment.

docs/frontend/react-hooks.mdx (4)

Line range hint 31-85: LGTM! Well-documented authentication options.

The authentication section clearly explains both direct token usage and provider-based approaches, with proper TypeScript types and comprehensive examples.


Line range hint 241-253: LGTM! Clear explanation of hook styles.

Good explanation of the differences between SWR and Realtime hooks, with helpful guidance about rate limits and API behavior.


611-685: LGTM! Comprehensive SWR hooks documentation.

The section provides clear examples, proper TypeScript types, and well-documented options and return values.


687-797: LGTM! Well-documented trigger hooks.

Excellent documentation of the trigger hooks with proper TypeScript types, clear examples, and stream handling.

Comment on lines +594 to +597
const { runs, error } = useRealtimeRunsWithStreams(tag, {
accessToken: publicAccessToken,
experimental_throttleInMs: 1000, // Throttle updates to once per second
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect parameter usage in the example.

The example uses a tag parameter that wasn't defined in the component props. It should use runId instead.

-  const { runs, error } = useRealtimeRunsWithStreams(tag, {
+  const { runs, error } = useRealtimeRunsWithStreams(runId, {

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +195 to +199
const completion = await openaiSDK.chat.completions.create({
messages: [{ role: "user", content: prompt }],
model: "gpt-3.5-turbo",
stream: true,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use the model parameter in OpenAI API call

The model parameter passed into the function is not used in the API call; instead, "gpt-3.5-turbo" is hardcoded. Update the code to use the model parameter to respect the input.

Apply this diff to fix the issue:

           messages: [{ role: "user", content: prompt }],
-          model: "gpt-3.5-turbo",
+          model: model,
           stream: true,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const completion = await openaiSDK.chat.completions.create({
messages: [{ role: "user", content: prompt }],
model: "gpt-3.5-turbo",
stream: true,
});
const completion = await openaiSDK.chat.completions.create({
messages: [{ role: "user", content: prompt }],
model: model,
stream: true,
});

Comment on lines +6 to +10
import OpenAI from "openai";

const openaiSDK = new OpenAI({
apiKey: process.env.OPENAI_API_KEY,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for missing OpenAI API key

To prevent runtime errors when OPENAI_API_KEY is missing, consider adding a check to ensure that process.env.OPENAI_API_KEY is defined before creating the OpenAI SDK instance.

Apply this diff to add the error handling:

+if (!process.env.OPENAI_API_KEY) {
+  throw new Error("OPENAI_API_KEY environment variable is not set.");
+}
 const openaiSDK = new OpenAI({
   apiKey: process.env.OPENAI_API_KEY,
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import OpenAI from "openai";
const openaiSDK = new OpenAI({
apiKey: process.env.OPENAI_API_KEY,
});
import OpenAI from "openai";
if (!process.env.OPENAI_API_KEY) {
throw new Error("OPENAI_API_KEY environment variable is not set.");
}
const openaiSDK = new OpenAI({
apiKey: process.env.OPENAI_API_KEY,
});

Comment on lines +153 to +183
export const fetchStream = schemaTask({
id: "fetch-stream",
description: "Stream data from fetch",
schema: z.object({
url: z.string().url(),
}),
run: async ({ url }) => {
logger.info("Streaming response", { url });

const response = await fetch(url);

if (!response.body) {
throw new Error("Response body is not readable");
}

const stream = await metadata.stream(
"fetch",
response.body.pipeThrough(new TextDecoderStream())
);

let text = "";

for await (const chunk of stream) {
logger.log("Received chunk", { chunk });

text += chunk;
}

return { text };
},
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure fetch and TextDecoderStream are available in Node.js environment

The fetch API and TextDecoderStream are used, which require Node.js 18+ or appropriate polyfills. Ensure that your runtime environment supports these APIs, or consider importing necessary packages.

If fetch is not globally available, you can import it:

+import fetch from 'node-fetch';

For TextDecoderStream, if it's not available, consider alternative methods to decode the stream, such as using the ReadableStream with a custom decoder or leveraging the StringDecoder from the string_decoder module.

Committable suggestion skipped: line range outside the PR's diff.

Copy link

pkg-pr-new bot commented Nov 20, 2024

@trigger.dev/react-hooks

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/react-hooks@1486

@trigger.dev/rsc

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/rsc@1486

@trigger.dev/sdk

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/sdk@1486

@trigger.dev/core

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/core@1486

@trigger.dev/build

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/build@1486

trigger.dev

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev@1486

commit: 8b68a82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant