chore(core): remove dead code, unnecessary casts, and redundant async#38149
Merged
deyaaeldeen merged 6 commits intomainfrom Apr 17, 2026
Merged
chore(core): remove dead code, unnecessary casts, and redundant async#38149deyaaeldeen merged 6 commits intomainfrom
deyaaeldeen merged 6 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Removes dead code, redundant async usage, and unnecessary casts/type aliases across core packages, aiming to simplify implementations and tighten typing.
Changes:
- Removed unused helpers/types and stale declarations across multiple core packages.
- Simplified type handling (e.g.,
Resolved<T>→Awaited<T>, narrower type guards, fewer casts). - Removed redundant
asyncwrappers that only forwarded promises and made small micro-optimizations (e.g.,Setmembership).
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/core/core-xml/src/xml.ts | Simplifies root key extraction when includeRoot is false. |
| sdk/core/core-util/src/typeGuards.ts | Tightens objectHasProperty implementation with explicit null check. |
| sdk/core/core-tracing/src/tracingClient.ts | Replaces Resolved with Awaited, removes redundant Promise.resolve, and simplifies object literals. |
| sdk/core/core-tracing/src/interfaces.ts | Deprecates Resolved in favor of built-in Awaited. |
| sdk/core/core-tracing/review/core-tracing-node.api.md | Updates public API surface to mark Resolved deprecated and uses Awaited. |
| sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts | Removes unsafe cast on WWW-Authenticate header retrieval. |
| sdk/core/core-paging/src/getPagedAsyncIterator.ts | Removes optional chaining on a guaranteed pagedResult reference. |
| sdk/core/core-lro/src/poller/poller.ts | Removes redundant async from progress event handler. |
| sdk/core/core-lro/src/http/operation.ts | Removes redundant async wrapper around lro.sendPollRequest. |
| sdk/core/core-http-compat/src/util.ts | Replaces passthrough prop list with a Set; makes HttpHeaders non-exported. |
| sdk/core/core-http-compat/src/policies/disableKeepAlivePolicy.ts | Removes redundant async from policy sendRequest. |
| sdk/core/core-client/src/urlHelpers.ts | Refactors URL path/search scope construction. |
| sdk/core/core-client/src/serviceClient.ts | Removes redundant async in sendRequest; adjusts credential scope error condition. |
| sdk/core/core-client/src/serializationPolicy.ts | Removes redundant async from policy sendRequest. |
| sdk/core/core-client/src/authorizeRequestOnTenantChallenge.ts | Fixes scope URL construction using URL base resolution. |
| sdk/core/core-client-rest/src/restError.ts | Replaces non-null assertion with safer cast for forwarding to tsp helper. |
| sdk/core/core-client-rest/src/keyCredentialAuthenticationPolicy.ts | Removes redundant async from policy sendRequest. |
| sdk/core/core-client-rest/src/clientHelpers.ts | Removes cached HTTP client dead code; tightens isKeyCredential parameter type. |
| sdk/core/core-client-rest/src/apiVersionPolicy.ts | Simplifies query param presence check using searchParams.size. |
| sdk/core/core-auth/src/tokenCredential.ts | Removes unused internal token-type helper functions. |
| sdk/core/core-amqp/src/util/utils.ts | Removes dead types/helpers; tightens parseConnectionString return typing. |
| sdk/core/abort-controller/src/index.ts | Removes stale global Event declaration. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
04d2aef to
6ceaa01
Compare
9ec70fd to
580cea9
Compare
580cea9 to
4ea1fd2
Compare
- Remove unused AsyncLockOptions, Timeout class (core-amqp) - Remove unused isBearerToken, isPopToken helpers (core-auth) - Remove unused getCachedDefaultHttpsClient (core-client-rest) - Remove stale global Event declaration (abort-controller) - Replace `as any` casts with proper type narrowing - Remove unnecessary `async` on non-awaiting functions - Simplify Resolved<T> to Awaited<T> (core-tracing) - Use Set for passthrough props lookup (core-http-compat) - Make HttpHeaders class non-exported (core-http-compat) - Simplify XML root key extraction (core-xml) - Fix URL scope construction (core-client) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4ea1fd2 to
d2d2cf8
Compare
xirzec
reviewed
Apr 15, 2026
xirzec
reviewed
Apr 15, 2026
Member
xirzec
left a comment
There was a problem hiding this comment.
Mostly good, though I have some questions.
d2d2cf8 to
71e3db9
Compare
- Restore async on handleProgressEvents to preserve microtask timing - Restore original objectHasProperty (no function support needed) - Remove associated function tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
71e3db9 to
6a684ca
Compare
- ndJsonPolicy: Remove unreachable Array.isArray guard after JSON.parse on a string starting with '[' (JSON spec guarantees array result) - tokenCycler: Replace optional chaining with explicit null guard in shouldRefresh, making intent clearer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
deyaaeldeen
pushed a commit
that referenced
this pull request
Apr 16, 2026
The ndJsonPolicy and tokenCycler simplifications have been moved to the chore/core-src-cleanup branch (PR #38149). This PR now contains only test additions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xirzec
reviewed
Apr 16, 2026
Member
xirzec
left a comment
There was a problem hiding this comment.
Pretty sure the tokenCycler change is incorrect
The public API re-exports calculateRetryDelay from index.ts which delegates to @typespec/ts-http-runtime. The implementation in delay.ts was never called. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Apr 17, 2026
xirzec
approved these changes
Apr 17, 2026
deyaaeldeen
pushed a commit
that referenced
this pull request
Apr 18, 2026
randomNumberFromInterval, executePromisesSequentially, isIotHubConnectionString, and Timeout were removed in #38149. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
deyaaeldeen
added a commit
that referenced
this pull request
Apr 18, 2026
…#38149) ## Summary Remove dead code, unnecessary type casts, and redundant `async` keywords across core packages. ### Dead code removal - `AsyncLockOptions` interface, `Timeout` class (core-amqp) - `isBearerToken`, `isPopToken` helpers (core-auth) - `getCachedDefaultHttpsClient`, unused `HttpClient` import (core-client-rest) - Stale `declare global { interface Event {} }` (abort-controller) ### Cast removal / type narrowing - `as any` → `as ParsedOutput` in `parseConnectionString` (core-amqp) - `{ [k: string]: string }` → `Record` (core-amqp) - `as string` → `?? ""` for CAE challenge header (core-rest-pipeline) - `isKeyCredential` param `any` → `unknown` with proper guard (core-client-rest) - `objectHasProperty` tightened with null check (core-util) ### Redundant async removal - `sendRequest`, `sendPolicy` handlers that just return a promise without awaiting (core-client, core-client-rest, core-http-compat, core-lro) ### Simplify defensive logic - **ndJsonPolicy**: Remove unreachable `Array.isArray` guard after `JSON.parse` on a string starting with `[` (JSON spec guarantees array result) - **tokenCycler**: Replace optional chaining with explicit null guard in `shouldRefresh`, making intent clearer ### Other improvements - `Resolved` simplified to `Awaited` with deprecation notice (core-tracing) - `HttpHeaders` class made non-exported (core-http-compat, was already not in public API) - `Set` for passthrough props lookup (core-http-compat) - XML root key extraction simplified (core-xml) - URL scope construction fix (core-client) - Optional chaining removed on guaranteed field (core-paging) --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Deyaaeldeen Almahallawi <deyaa@microsoft.com>
deyaaeldeen
pushed a commit
that referenced
this pull request
Apr 18, 2026
randomNumberFromInterval, executePromisesSequentially, isIotHubConnectionString, and Timeout were removed in #38149. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
deyaaeldeen
pushed a commit
that referenced
this pull request
Apr 18, 2026
randomNumberFromInterval, executePromisesSequentially, isIotHubConnectionString, and Timeout were removed in #38149. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Remove dead code, unnecessary type casts, and redundant
asynckeywords across core packages.Dead code removal
AsyncLockOptionsinterface,Timeoutclass (core-amqp)isBearerToken,isPopTokenhelpers (core-auth)getCachedDefaultHttpsClient, unusedHttpClientimport (core-client-rest)declare global { interface Event {} }(abort-controller)Cast removal / type narrowing
as any→as ParsedOutputinparseConnectionString(core-amqp){ [k: string]: string }→Record(core-amqp)as string→?? ""for CAE challenge header (core-rest-pipeline)isKeyCredentialparamany→unknownwith proper guard (core-client-rest)objectHasPropertytightened with null check (core-util)Redundant async removal
sendRequest,sendPolicyhandlers that just return a promise without awaiting (core-client, core-client-rest, core-http-compat, core-lro)Simplify defensive logic
Array.isArrayguard afterJSON.parseon a string starting with[(JSON spec guarantees array result)shouldRefresh, making intent clearerOther improvements
Resolvedsimplified toAwaitedwith deprecation notice (core-tracing)HttpHeadersclass made non-exported (core-http-compat, was already not in public API)Setfor passthrough props lookup (core-http-compat)