Skip to content

Deprecate ensureReplicas/disableReplicas.#6638

Merged
jtolio merged 1 commit intomainfrom
jolio/deprecate-ensureReplicas
Apr 24, 2026
Merged

Deprecate ensureReplicas/disableReplicas.#6638
jtolio merged 1 commit intomainfrom
jolio/deprecate-ensureReplicas

Conversation

@jtolio
Copy link
Copy Markdown
Member

@jtolio jtolio commented Apr 22, 2026

In cases where typescript annotations mention the old methods, we should explain what is replacing them.

Once #6649 is merged this PR should be retargeted to main.

@jtolio jtolio requested review from a team as code owners April 22, 2026 15:18
@jtolio jtolio requested review from dario-piotrowicz and justin-mp and removed request for a team April 22, 2026 15:18
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR adds @deprecated annotations for ensureReplicas() and disableReplicas() on DurableObjectStorage, pointing users to configureReadReplication() instead.

Issue: @deprecated tags are inside plain comments, not JSDoc comments — TypeScript will not recognize them.

The annotation strings start with a space (` @deprecated...`), which causes ts.addSyntheticLeadingComment to emit /* @deprecated ... */ (plain comment). TypeScript only processes @deprecated tags inside /** */ (JSDoc) comments. Users will not see strikethrough in their IDEs.

The fix is to prefix the strings with * instead of a space, producing /** @deprecated ... */. The pre-existing primary entry has the same bug.


This review was generated by an AI assistant and may contain inaccuracies.

Comment thread types/src/cloudflare.ts Outdated
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 22, 2026

I'm Bonk, and I've done a quick review of your PR.

This PR adds @deprecated annotations for ensureReplicas() and disableReplicas() on DurableObjectStorage, pointing users to configureReadReplication() instead.

One issue found: The @deprecated tags are inside plain comments (/* */) instead of JSDoc comments (/** */). TypeScript only recognizes @deprecated inside JSDoc comments, so users won't see strikethrough in their IDEs. The fix is to change the annotation strings in types/src/cloudflare.ts to start with * instead of a space. I posted a suggestion comment with the fix (which also fixes the pre-existing bug on the primary entry).

github run

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 22, 2026

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing jolio/deprecate-ensureReplicas (2d45197) with jolio/move-config-read-replication (3a820d2)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.50%. Comparing base (3a820d2) to head (2d45197).

Additional details and impacted files
@@                         Coverage Diff                         @@
##           jolio/move-config-read-replication    #6638   +/-   ##
===================================================================
  Coverage                               66.50%   66.50%           
===================================================================
  Files                                     405      405           
  Lines                                  115976   115976           
  Branches                                19409    19409           
===================================================================
  Hits                                    77133    77133           
  Misses                                  27265    27265           
  Partials                                11578    11578           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jtolio jtolio force-pushed the jolio/add-primaryStub branch 2 times, most recently from 1d15aff to da3c505 Compare April 22, 2026 18:42
@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from d92dd25 to 765f711 Compare April 22, 2026 18:48
Copy link
Copy Markdown
Contributor

@justin-mp justin-mp left a comment

Choose a reason for hiding this comment

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

Note that you don't actually need to base this on top of the other branch, as it doesn't depend on that branch for anything. You could rebase on top of main and it would apply just fine!

@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from 765f711 to 15cc016 Compare April 23, 2026 20:13
@jtolio jtolio requested review from a team as code owners April 23, 2026 20:13
@jtolio jtolio changed the base branch from jolio/add-primaryStub to main April 23, 2026 20:13
@jtolio
Copy link
Copy Markdown
Member Author

jtolio commented Apr 23, 2026

Rebase and retarget branch

@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from 15cc016 to 756c7d5 Compare April 23, 2026 22:07
@jtolio jtolio changed the base branch from main to jolio/add-ctx-primaryStub April 23, 2026 22:07
@jtolio
Copy link
Copy Markdown
Member Author

jtolio commented Apr 23, 2026

Note that you don't actually need to base this on top of the other branch, as it doesn't depend on that branch for anything. You could rebase on top of main and it would apply just fine!

Logically it's not dependent, but sadly it is syntactically/git conflict dependent, due to either this or the change below it adding a new stanza to types/src/cloudflare.ts

@jtolio jtolio force-pushed the jolio/add-ctx-primaryStub branch from 56a2896 to 0864429 Compare April 23, 2026 22:52
@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from 756c7d5 to 82cc84b Compare April 23, 2026 22:52
@jtolio jtolio force-pushed the jolio/add-ctx-primaryStub branch from 0864429 to 857b457 Compare April 24, 2026 14:14
@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from 82cc84b to e48fcfb Compare April 24, 2026 14:14
Base automatically changed from jolio/add-ctx-primaryStub to jolio/move-config-read-replication April 24, 2026 14:14
@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from e48fcfb to 5dccfbf Compare April 24, 2026 14:36
@jtolio jtolio force-pushed the jolio/move-config-read-replication branch 2 times, most recently from 950bce5 to d668e2b Compare April 24, 2026 14:38
@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from 5dccfbf to 343e809 Compare April 24, 2026 14:38
@jtolio jtolio force-pushed the jolio/move-config-read-replication branch from d668e2b to 0649f75 Compare April 24, 2026 15:27
@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from 343e809 to 2b8f20e Compare April 24, 2026 15:27
In cases where typescript annotations mention the
old methods, we should explain what is replacing them.

X-Dest-Branch: jolio/deprecate-ensureReplicas
@jtolio jtolio force-pushed the jolio/move-config-read-replication branch from 0649f75 to 3a820d2 Compare April 24, 2026 18:23
@jtolio jtolio force-pushed the jolio/deprecate-ensureReplicas branch from 2b8f20e to 2d45197 Compare April 24, 2026 18:23
Base automatically changed from jolio/move-config-read-replication to main April 24, 2026 19:02
@jtolio jtolio enabled auto-merge (rebase) April 24, 2026 19:26
@jtolio jtolio merged commit f7c209c into main Apr 24, 2026
20 of 35 checks passed
@jtolio jtolio deleted the jolio/deprecate-ensureReplicas branch April 24, 2026 19:35
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.

4 participants