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

chore: update cdn url #1123

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

chore: update cdn url #1123

wants to merge 3 commits into from

Conversation

juliusmarminge
Copy link
Collaborator

@juliusmarminge juliusmarminge commented Jan 9, 2025

Summary by CodeRabbit

  • Infrastructure

    • Updated file and image URLs from utfs.io to a new domain format <APP_ID>.ufs.sh
    • Added new environment variable EXPO_PUBLIC_UPLOADTHING_APP_ID for dynamic URL generation
  • Documentation

    • Updated documentation to reflect new file access URL patterns
    • Added notes about deprecated URL formats and recommended practices
  • Testing

    • Updated test cases to use new URL construction methods
    • Modified URL handling in various test suites to align with new infrastructure

Copy link

changeset-bot bot commented Jan 9, 2025

⚠️ No Changeset found

Latest commit: 0757a05

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.

This PR includes no changesets

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

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Jan 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-uploadthing ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 9, 2025 6:52pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
legacy-docs-uploadthing ⬜️ Ignored (Inspect) Visit Preview Jan 9, 2025 6:52pm

Copy link
Contributor

coderabbitai bot commented Jan 9, 2025

Walkthrough

This pull request involves a comprehensive update to the URL structure for file handling across multiple components of the application. The primary change is replacing the hostname from "utfs.io" to a new format using "<APP_ID>.ufs.sh" for file URLs. This modification affects configuration files, documentation, test suites, and example applications, ensuring consistent URL generation and file access across the project.

Changes

File Change Summary
docs/next.config.js Updated remotePatterns with new hostname and pathname for image processing
docs/src/app/(api)/api/og/blog/route.tsx, docs/src/app/(api)/api/og/docs/route.tsx Modified background image URL to use new hostname
docs/src/app/(docs)/concepts/regions-acl/page.mdx, docs/src/app/(docs)/working-with-files/page.mdx Updated documentation to reflect new file URL format and added clarification about URL changes
examples/minimal-expo/.env.example Added new environment variable EXPO_PUBLIC_UPLOADTHING_APP_ID
examples/minimal-expo/app/f/[key].tsx Updated file URL construction to use environment variable
packages/react/test/*, packages/uploadthing/test/* Replaced UTFS_IO_URL with UFS_HOST, updated URL patterns in test cases
playground/middleware.ts Commented out session validation middleware

Suggested reviewers

  • markflorkowski

Possibly related PRs

Finishing Touches

  • 📝 Generate Docstrings

🪧 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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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.

Copy link
Contributor

github-actions bot commented Jan 9, 2025

📦 Bundle size comparison

Bundle Size (gzip) Visualization
Main 30.07KB See Treemap 📊
PR (6977259) 30.07KB See Treemap 📊
Diff No change

Copy link

pkg-pr-new bot commented Jan 9, 2025

Open in Stackblitz

More templates

npm i https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/react@1123
npm i https://pkg.pr.new/pingdotgg/uploadthing@1123

commit: 0757a05

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: 7

🧹 Nitpick comments (5)
docs/next.config.js (1)

22-23: Consider using environment variables for the hostname

The hostname is currently hardcoded with a specific app ID. Consider using environment variables to make the configuration more portable across different environments:

-        hostname: "s40vlb3kca.ufs.sh",
+        hostname: `${process.env.NEXT_PUBLIC_UPLOADTHING_APP_ID}.ufs.sh`,
packages/react/test/upload-dropzone.browser.test.tsx (1)

46-46: Use a consistent test constant for the app ID

The hardcoded "app-1" in the URL should be replaced with a test constant to maintain consistency and make tests more maintainable:

-      return HttpResponse.json({ url: "https://app-1.ufs.sh/f/" + params.key });
+      return HttpResponse.json({ url: `https://${TEST_APP_ID}.ufs.sh/f/${params.key}` });

Consider adding this constant at the top of the file:

const TEST_APP_ID = "app-1";
packages/uploadthing/test/sdk.live.test.ts (1)

Line range hint 270-270: Update the commented URL to use the new format.

Even though this is commented code, it should be updated to use the new URL format for consistency.

-  //       url: "https://app-1.ufs.sh/f/abc-123.txt",
+  //       url: `https://${testToken.decoded.appId}.${UFS_HOST}/f/abc-123.txt`,
docs/src/app/(docs)/working-with-files/page.mdx (2)

35-35: Fix grammatical error in setup instructions.

Change "setup" to "set up" when used as a verb phrase.

-to setup image optimization allow filtering in Next.js that only allows
+to set up image optimization allow filtering in Next.js that only allows
🧰 Tools
🪛 LanguageTool

[uncategorized] ~35-~35: Possible missing preposition found.
Context: ...mple of how to setup image optimization allow filtering in Next.js that only allows o...

(AI_HYDRA_LEO_MISSING_TO)


Line range hint 15-46: Consider simplifying the file access documentation.

Based on previous feedback, this section could be more concise by focusing on three key points:

  1. Don't use storage URLs
  2. Use the correct URL format: https://<appid>.ufs.sh/f/<fileKey | customId>
  3. Next.js configuration example
🧰 Tools
🪛 LanguageTool

[uncategorized] ~35-~35: Possible missing preposition found.
Context: ...mple of how to setup image optimization allow filtering in Next.js that only allows o...

(AI_HYDRA_LEO_MISSING_TO)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1a1cf4 and 0757a05.

📒 Files selected for processing (15)
  • docs/next.config.js (1 hunks)
  • docs/src/app/(api)/api/og/blog/route.tsx (1 hunks)
  • docs/src/app/(api)/api/og/docs/route.tsx (1 hunks)
  • docs/src/app/(docs)/concepts/regions-acl/page.mdx (1 hunks)
  • docs/src/app/(docs)/working-with-files/page.mdx (2 hunks)
  • examples/minimal-expo/.env.example (1 hunks)
  • examples/minimal-expo/app/f/[key].tsx (1 hunks)
  • packages/react/test/upload-button.browser.test.tsx (1 hunks)
  • packages/react/test/upload-dropzone.browser.test.tsx (1 hunks)
  • packages/uploadthing/test/__test-helpers.ts (4 hunks)
  • packages/uploadthing/test/client.browser.test.ts (4 hunks)
  • packages/uploadthing/test/request-handler.test.ts (5 hunks)
  • packages/uploadthing/test/sdk.live.test.ts (8 hunks)
  • packages/uploadthing/test/sdk.test.ts (6 hunks)
  • playground/middleware.ts (2 hunks)
✅ Files skipped from review due to trivial changes (3)
  • docs/src/app/(api)/api/og/docs/route.tsx
  • packages/react/test/upload-button.browser.test.tsx
  • docs/src/app/(api)/api/og/blog/route.tsx
🧰 Additional context used
🪛 GitHub Actions: CI
packages/uploadthing/test/sdk.live.test.ts

[error] 59-59: Test 'should upload a file' failed due to URL mismatch. Expected URL pattern '/^https://fr0hfwpst1.ufs.sh/f/.+$/' but received 'https://utfs.io/f/' URL pattern


[error] 94-94: Test 'should upload a private file' failed due to URL pattern mismatch in appUrl and url fields


[error] 124-124: Test 'should upload a file from a url' failed due to incorrect URL format in response


[error] 147-147: Test 'should rename a file with fileKey' failed due to URL pattern mismatch


[error] 190-190: Test 'should rename a file with customId' failed due to incorrect URL format


[error] 231-231: Test 'should update ACL' failed due to URL pattern mismatch in response


[error] 279-279: Test 'should have correct usage info' failed. Expected totalBytes: 0, appTotalBytes: 0, filesUploaded: -1 but received different values

🪛 LanguageTool
docs/src/app/(docs)/working-with-files/page.mdx

[uncategorized] ~35-~35: Possible missing preposition found.
Context: ...mple of how to setup image optimization allow filtering in Next.js that only allows o...

(AI_HYDRA_LEO_MISSING_TO)

🔇 Additional comments (19)
playground/middleware.ts (1)

12-16: ⚠️ Potential issue

Verify if commenting out session validation is intentional

The commented-out session validation logic appears unrelated to the CDN URL update. If this is an intentional change, please document the rationale. Otherwise, consider reverting these changes to maintain security.

examples/minimal-expo/app/f/[key].tsx (1)

13-13: LGTM! Good use of environment variables

The implementation correctly uses environment variables for the hostname, making it configurable across different environments.

packages/uploadthing/test/__test-helpers.ts (5)

34-37: LGTM! The URL configuration aligns with the PR objective.

The constant UFS_HOST correctly handles both staging and production environments.


43-44: LGTM! The URL pattern matches the new format.

The pattern correctly validates URLs in the format https://<appId>.<UFS_HOST>/f/<fileKey>.


88-91: LGTM! The HTTP handler matches the new URL format.

The handler correctly intercepts requests to the new URL pattern.


101-102: LGTM! The ingest handler uses the new URL format.

Both url and appUrl are correctly constructed using the new URL structure.


116-116: LGTM! The file access handler uses the new URL format.

The URL is correctly constructed with the new format and preserves the S3 query parameters.

packages/uploadthing/test/sdk.live.test.ts (1)

7-7: LGTM! The import statement is updated correctly.

The import includes the new UFS_HOST constant.

packages/uploadthing/test/client.browser.test.ts (3)

143-144: LGTM! The URL expectations match the new format.

The test correctly validates URLs using the new pattern.


187-188: LGTM! The URL expectations in custom headers test match the new format.

The test correctly validates URLs using the new pattern.


222-223: LGTM! The URL expectations in async headers test match the new format.

The test correctly validates URLs using the new pattern.

packages/uploadthing/test/request-handler.test.ts (5)

16-16: LGTM! The import statement is updated correctly.

The import includes the new UFS_HOST constant.


413-414: LGTM! The URLs in upload complete test use the new format.

Both url and appUrl are correctly constructed using the new URL structure.


450-451: LGTM! The URLs in upload complete callback use the new format.

Both url and appUrl are correctly constructed using the new URL structure.


463-464: LGTM! The URLs in missing signature test use the new format.

Both url and appUrl are correctly constructed using the new URL structure.


496-497: LGTM! The URLs in invalid signature test use the new format.

Both url and appUrl are correctly constructed using the new URL structure.

packages/uploadthing/test/sdk.test.ts (1)

53-54: LGTM! Test cases updated correctly for new URL format.

The URL construction in test assertions has been consistently updated to use the new format with ${testToken.decoded.appId}.${UFS_HOST}.

Also applies to: 129-130, 263-264, 301-302, 324-325

examples/minimal-expo/.env.example (1)

2-3: LGTM! Environment variable added for new URL structure.

The EXPO_PUBLIC_UPLOADTHING_APP_ID variable is correctly prefixed for Expo's public environment variables.

docs/src/app/(docs)/concepts/regions-acl/page.mdx (1)

53-53: LGTM! Documentation updated for new URL format.

The URL format has been correctly updated to use the new APP_ID.ufs.sh structure.

Comment on lines +67 to +68
url: expect.stringMatching(fileUrlPattern(appId)),
appUrl: expect.stringMatching(fileUrlPattern(appId)),
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 the failing test by updating URL expectations.

The test is failing due to URL pattern mismatch. According to the pipeline failure:

Test 'should upload a file' failed due to URL mismatch. Expected URL pattern '/^https:\/\/fr0hfwpst1.ufs.sh\/f\/.+$/' but received 'https://utfs.io/f/' URL pattern

Comment on lines +102 to +103
url: expect.stringMatching(fileUrlPattern(appId)),
appUrl: expect.stringMatching(fileUrlPattern(appId)),
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 the failing test for private file upload.

The test is failing due to URL pattern mismatch according to the pipeline failure.

Comment on lines +132 to +133
url: expect.stringMatching(fileUrlPattern(appId)),
appUrl: expect.stringMatching(fileUrlPattern(appId)),
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 the failing test for URL upload.

The test is failing due to incorrect URL format in response according to the pipeline failure.

Comment on lines +156 to +157
url: expect.stringMatching(fileUrlPattern(appId)),
appUrl: expect.stringMatching(fileUrlPattern(appId)),
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 the failing test for file rename.

The test is failing due to URL pattern mismatch according to the pipeline failure.

Comment on lines +198 to +199
url: expect.stringMatching(fileUrlPattern(appId)),
appUrl: expect.stringMatching(fileUrlPattern(appId)),
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 the failing test for custom ID.

The test is failing due to incorrect URL format according to the pipeline failure.

Comment on lines +239 to +240
url: expect.stringMatching(fileUrlPattern(appId)),
appUrl: expect.stringMatching(fileUrlPattern(appId)),
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 the failing test for ACL update.

The test is failing due to URL pattern mismatch according to the pipeline failure.

Comment on lines +262 to +264
const response = await fetch(
`https://${testToken.decoded.appId}.${UFS_HOST}/f/${someFile.key}`,
);
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 the failing test for file deletion.

The test is failing. The usage info test indicates incorrect values for totalBytes and filesUploaded according to the pipeline failure.

@juliusmarminge
Copy link
Collaborator Author

juliusmarminge commented Jan 9, 2025

once the infra has been deployed the live tests will pass

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

Successfully merging this pull request may close these issues.

2 participants