refactor(workflow): rename defineWaitPoint/defineWaitPoints to createWaitPoint/createWaitPoints#1556
Merged
Conversation
…WaitPoint/createWaitPoints These functions create runtime instances with .wait()/.resolve() methods that call the platform API, so they follow the create* naming convention. Also document the define* vs create* rule in sdk-internals agent rules.
🦋 Changeset detectedLatest commit: 59ef383 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
commit: |
This comment has been minimized.
This comment has been minimized.
Rename defineWaitPoint and defineWaitPoints to createWaitPoint and createWaitPoints in user code. Handles both non-aliased imports (renames the specifier and all body references) and aliased imports (renames only the imported name, preserving the alias). Includes fixture tests for: basic rename, aliased imports, no-op cases (no matching imports, no SDK import).
This comment has been minimized.
This comment has been minimized.
Skip body-identifier rename for any name that is also declared as a function or variable in the file body, preventing incorrect rewrites of locals that happen to share the SDK import name (e.g. a helper that re-declares defineWaitPoints in an inner scope). Adds a local-shadow fixture to verify the import specifier is still renamed while the shadowed body usages are left unchanged.
This comment has been minimized.
This comment has been minimized.
Replace the over-broad needsBodyRename.delete() guard with a per-scope byte-range map. For each local declaration that shadows an SDK import name, record the byte range of its containing statement_block (or program for top-level). Body identifiers are only skipped when their position falls within one of those ranges, so top-level SDK usages are correctly renamed even when the same name is re-declared in a nested function. Adds a nested-shadow fixture to verify: top-level defineWaitPoints is renamed to createWaitPoints while the re-declared local inside a helper function and its calls are left unchanged.
This comment has been minimized.
This comment has been minimized.
Add for_statement and for_in_statement to the scope-kind check when walking up the AST to find the containing scope of a local declaration. Without this, a variable_declarator in a for-loop initializer (for (let defineWaitPoints = ...)) walked all the way up to program, causing the entire file to be treated as shadowed and preventing top-level SDK usages from being renamed. Adds a for-loop-shadow fixture to verify that top-level defineWaitPoints is renamed while the loop counter and body references are left unchanged.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dqn
reviewed
Jun 26, 2026
| }); | ||
| for (const decl of localDecls) { | ||
| if (isInsideImportStatement(decl)) continue; | ||
| const nameChild = decl.children().find((c: SgNode) => c.kind() === "identifier"); |
Contributor
There was a problem hiding this comment.
Destructured local bindings such as const { defineWaitPoints } = source are not recorded as shadows, so the transform can rewrite their local calls to createWaitPoints() after renaming the SDK import.
| } | ||
| } | ||
|
|
||
| const identifiers = root.findAll({ rule: { kind: "identifier" } }); |
Contributor
There was a problem hiding this comment.
Object-literal shorthand usages such as const api = { defineWaitPoints } are not renamed, so after the import becomes createWaitPoints the shorthand still references an undefined defineWaitPoints.
…ng shadows in wait-point-rename
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dqn
approved these changes
Jun 26, 2026
dqn
left a comment
dqn
left a comment
Contributor
There was a problem hiding this comment.
LGTM! Please just fix the conflicts
Code Metrics Report (packages/sdk)
Details | | main (04c2ebb) | #1556 (840d4e4) | +/- |
|--------------------|----------------|-----------------|-------|
+ | Coverage | 69.7% | 70.7% | +0.9% |
| Files | 422 | 417 | -5 |
| Lines | 15081 | 15098 | +17 |
+ | Covered | 10516 | 10676 | +160 |
+ | Code to Test Ratio | 1:0.4 | 1:0.4 | +0.0 |
| Code | 102339 | 102537 | +198 |
+ | Test | 45593 | 46347 | +754 |Code coverage of files in pull request scope (71.5% → 76.1%)DetailsSDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary