Skip to content

[rails__activestorage] Fix callback error type to string - #75747

Open
vivekmiyani wants to merge 1 commit into
DefinitelyTyped:masterfrom
vivekmiyani:rails-activestorage-error-string
Open

vivekmiyani wants to merge 1 commit into
DefinitelyTyped:masterfrom
vivekmiyani:rails-activestorage-error-string

Conversation

@vivekmiyani

Copy link
Copy Markdown

If changing an existing definition:

Active Storage's JS passes error string to the create callbacks, not Error. With the current types, error.message type-checks but is undefined at runtime.

This changes the callback error type from Error to string | null in DirectUpload#create, BlobUpload#create and FileChecksum.create.

@typescript-automation

typescript-automation Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

@vivekmiyani Thank you for submitting this PR!

This is a live comment that I will keep updated.

1 package in this PR

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • 🕐 Most recent commit is approved by type definition owners or DT maintainers

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 75747,
  "author": "vivekmiyani",
  "headCommitOid": "0daef1cbb397af9974c46ddd701816af202a3779",
  "mergeBaseOid": "84da967d1a63c8b3cd1e9251b3b8fd207c100164",
  "lastPushDate": "2026-10-07T19:21:12.000Z",
  "lastActivityDate": "2026-10-07T19:21:12.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "tooManyCommits": false,
  "tooManyReviews": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "rails__activestorage",
      "version": "8.0",
      "kind": "edit",
      "files": [
        {
          "path": "types/rails__activestorage/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/rails__activestorage/rails__activestorage-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/rails__activestorage/src/blob_upload.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/rails__activestorage/src/file_checksum.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "imustafin",
        "cbothner"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [],
  "mainBotCommentID": 6045149653,
  "ciResult": "pass"
}

@typescript-automation typescript-automation Bot added the Popular package This PR affects a popular package (as counted by NPM download counts). label Oct 7, 2026
@typescript-automation

Copy link
Copy Markdown
Contributor

🔔 @imustafin @cbothner — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

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

Labels

Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

Status: Waiting for Code Reviews

Development

Successfully merging this pull request may close these issues.

1 participant