Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core-util] Create isNodeLike and isNodeRuntime #29086

Merged
merged 4 commits into from
Mar 27, 2024
Merged

Conversation

mpodwysocki
Copy link
Member

Packages impacted by this PR

  • @azure/core-util

Issues associated with this PR

Describe the problem that is addressed by this PR

Reverts the behavior of isNode checks to ensure that it can still return true for Bun and Deno. Introducing isNodeLike to also have the same behavior as isNode so we can deprecate the former function. We have also added isNodeRuntime which then checks for an explicit Node.js runtime.

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

The previous attempt was to create a isNodeLike and be opt-in, versus this PR which gives the old behavior and new opt-in for strict Node runtimes.

Are there test cases added in this PR? (If not, why?)

Provide a list of related PRs (if any)

Command used to generate this PR:**(Applicable only to SDK release request PRs)

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

@azure-sdk
Copy link
Collaborator

azure-sdk commented Mar 27, 2024

API change check

APIView has identified API level changes in this PR and created following API reviews.

@azure/core-util
@typespec/ts-http-runtime

Copy link
Member

@timovv timovv left a comment

Choose a reason for hiding this comment

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

Would you be able to port this to ts-http-runtime before you merge?

@mpodwysocki
Copy link
Member Author

Would you be able to port this to ts-http-runtime before you merge?

@timovv done

@mpodwysocki mpodwysocki enabled auto-merge (squash) March 27, 2024 19:36
@mpodwysocki mpodwysocki merged commit e9f9ff8 into main Mar 27, 2024
14 checks passed
@mpodwysocki mpodwysocki deleted the feat/node-like branch March 27, 2024 19:59
it("should return true", async function () {
assert.isFalse(isNode);
});
});

describe("isNodeRuntime (browser)", function () {
it("should return true", async function () {
Copy link
Member

Choose a reason for hiding this comment

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

Test names not accurate.

Copy link
Member

@witemple-msft witemple-msft left a comment

Choose a reason for hiding this comment

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

LGTM

@timovv timovv mentioned this pull request Apr 11, 2024
timovv added a commit that referenced this pull request Apr 16, 2024
### Packages impacted by this PR

- `@azure/core-rest-pipeline`
- `@typespec/ts-http-runtime`

### Describe the problem that is addressed by this PR

Use `isNodeLike` in favor of the deprecated `isNode` in Core packages
that depend on `core-util`.

### Provide a list of related PRs _(if any)_

- #29086
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants