Skip to content

refactor(storage): convert storageUrl to named args#9833

Closed
Light2Dark wants to merge 1 commit into
mainfrom
Light2Dark/refactor-storageurl-named-args
Closed

refactor(storage): convert storageUrl to named args#9833
Light2Dark wants to merge 1 commit into
mainfrom
Light2Dark/refactor-storageurl-named-args

Conversation

@Light2Dark

@Light2Dark Light2Dark commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

📝 Summary

Split out from #9708.

Pure refactor: converts storageUrl(protocol, rootPath, entryPath) to a single named-args object to improve readability at call sites. Satisfies typechecker.

📋 Pre-Review Checklist

  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Jun 9, 2026 9:33am

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Architecture diagram
sequenceDiagram
    participant Caller as Storage URL Caller
    participant types as types.ts
    participant URL as URL Constructor

    Note over Caller,URL: CHANGED: storageUrl now uses named params object

    Caller->>types: storageUrl({ protocol, rootPath, entryPath })
    types->>types: Destructure named args
    types->>types: Filter rootPath and entryPath by truthiness
    types->>types: Join filtered paths with "/"
    types->>types: Collapse multiple slashes via replaceAll
    types->>URL: new URL(`${protocol}://${joinedPath}`)
    URL-->>types: Return constructed URL
    types-->>Caller: Return URL object

    Note over Caller: Call sites now pass object literal<br/>instead of positional args
Loading

Re-trigger cubic

@Light2Dark Light2Dark marked this pull request as ready for review June 9, 2026 09:48
@Light2Dark Light2Dark added the internal A refactor or improvement that is not user facing label Jun 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the storageUrl helper in the frontend storage layer to take a single named-arguments object rather than positional parameters, aiming to improve readability and make argument ordering explicit.

Changes:

  • Updated storageUrl signature in frontend/src/core/storage/types.ts to accept a destructured object with { protocol, rootPath, entryPath }.
  • Updated unit tests to call storageUrl with named arguments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
frontend/src/core/storage/types.ts Refactors storageUrl to accept a named-args object.
frontend/src/core/storage/tests/types.test.ts Updates tests to match the new storageUrl call signature.

Comment on lines +35 to +43
export function storageUrl({
protocol,
rootPath,
entryPath,
}: {
protocol: string;
rootPath: string;
entryPath: string;
}): URL {
@Light2Dark Light2Dark closed this Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants