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.

Sign up for GitHub

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

[storage blob] Change from isNode to isNodeCompatible #29083

Closed
wants to merge 2 commits into from
Closed

[storage blob] Change from isNode to isNodeCompatible #29083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14377bf
[storage blob] Change from isNode to isNodeCompatible
mpodwysocki Mar 27, 2024
9c19ebc
[storage blob] Change from isNode to isNodeLike
mpodwysocki Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions sdk/core/core-util/review/core-util.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ export function isError(e: unknown): e is Error;
// @public
export const isNode: boolean;

// @public
export const isNodeLike: boolean;

// @public
export function isObject(input: unknown): input is UnknownObject;

Expand Down
5 changes: 5 additions & 0 deletions sdk/core/core-util/src/checkEnvironment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ export const isNode =
!isDeno &&
Copy link
Member

Choose a reason for hiding this comment

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

GH did not let me comment over the right line (69) but I wonder if we should mark isNode as @deprecated so package owners know to use isNodeCompatible instead

Copy link
Member Author

Choose a reason for hiding this comment

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

That's possible, @xirzec any thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Feels a bit odd to me having isDeno and isBun around while deprecating isNode. I wonder if we will ever run into a scenario in future where we will be wanting to check for Node specifically, intentionally excluding Deno/Bun?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think we should deprecate the existing one if we're reverting the behavior to match isNodeCompatible (really just exporting it twice with different names.)

If we think there's a case for "is actually node" then perhaps we can use a worse name for it? like isNodeRuntime ? And then clarify in the doc comments all the options

Copy link
Member

Choose a reason for hiding this comment

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

Oh if we're going to change the behavior of isNode to be the same as the new flag then I support deprecating but that doesn't seem to be what we're doing in this PR?

!isBun;

/**
* A constant that indicates whether the environment the code is running is a Node.JS compatible environment.
*/
export const isNodeLike = isNode || isDeno || isBun;

/**
* A constant that indicates whether the environment the code is running is in React-Native.
*/
Expand Down
1 change: 1 addition & 0 deletions sdk/core/core-util/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export {
isBrowser,
isBun,
isNode,
isNodeLike,
isDeno,
isReactNative,
isWebWorker,
Expand Down
26 changes: 13 additions & 13 deletions sdk/storage/storage-blob/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,19 @@
}
},
"dependencies": {
"@azure/abort-controller": "^1.0.0",
"@azure/core-auth": "^1.4.0",
"@azure/core-client": "^1.6.2",
"@azure/core-http-compat": "^2.0.0",
"@azure/core-lro": "^2.2.0",
"@azure/core-paging": "^1.1.1",
"@azure/core-rest-pipeline": "^1.10.1",
"@azure/core-tracing": "^1.0.0",
"@azure/core-util": "^1.6.1",
"@azure/core-xml": "^1.3.2",
"@azure/logger": "^1.0.0",
"@azure/abort-controller": "^1.1.0",
"@azure/core-auth": "^1.7.1",
"@azure/core-client": "^1.9.1",
"@azure/core-http-compat": "^2.1.1",
"@azure/core-lro": "^2.7.1",
"@azure/core-paging": "^1.6.1",
"@azure/core-rest-pipeline": "^1.15.1",
"@azure/core-tracing": "^1.1.1",
"@azure/core-util": "^1.8.1",
"@azure/core-xml": "^1.4.1",
"@azure/logger": "^1.1.1",
"events": "^3.0.0",
"tslib": "^2.2.0"
"tslib": "^2.6.2"
},
"devDependencies": {
"@azure/dev-tool": "^1.0.0",
Expand All @@ -149,6 +149,7 @@
"@types/chai": "^4.1.6",
"@types/mocha": "^10.0.0",
"@types/node": "^18.0.0",
"c8": "^9.1.0",
"chai": "^4.2.0",
"cross-env": "^7.0.2",
"dotenv": "^16.0.0",
Expand All @@ -166,7 +167,6 @@
"karma-mocha-reporter": "^2.2.5",
"karma-sourcemap-loader": "^0.3.8",
"mocha": "^10.0.0",
"c8": "^9.1.0",
"puppeteer": "^22.2.0",
"rimraf": "^5.0.5",
"source-map-support": "^0.5.9",
Expand Down
6 changes: 3 additions & 3 deletions sdk/storage/storage-blob/src/BlobBatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
PipelineResponse,
SendRequest,
} from "@azure/core-rest-pipeline";
import { isNode } from "@azure/core-util";
import { isNodeLike } from "@azure/core-util";
Copy link
Member

Choose a reason for hiding this comment

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

do we really need to adopt this right away? I think if we're reverting the behavior in core-util we should do the migration off the deprecated check in its own PR and we can unblock customers by shipping core-util with the fixed behavior

import { AnonymousCredential } from "./credentials/AnonymousCredential";
import { BlobClient, BlobDeleteOptions, BlobSetTierOptions } from "./Clients";
import { AccessTier } from "./generatedModels";
Expand Down Expand Up @@ -161,7 +161,7 @@ export class BlobBatch {

if (
typeof urlOrBlobClient === "string" &&
((isNode && credentialOrOptions instanceof StorageSharedKeyCredential) ||
((isNodeLike && credentialOrOptions instanceof StorageSharedKeyCredential) ||
credentialOrOptions instanceof AnonymousCredential ||
isTokenCredential(credentialOrOptions))
) {
Expand Down Expand Up @@ -265,7 +265,7 @@ export class BlobBatch {

if (
typeof urlOrBlobClient === "string" &&
((isNode && credentialOrTier instanceof StorageSharedKeyCredential) ||
((isNodeLike && credentialOrTier instanceof StorageSharedKeyCredential) ||
credentialOrTier instanceof AnonymousCredential ||
isTokenCredential(credentialOrTier))
) {
Expand Down
6 changes: 3 additions & 3 deletions sdk/storage/storage-blob/src/BlobDownloadResponse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
import { isNode } from "@azure/core-util";
import { BlobImmutabilityPolicyMode } from "./generatedModels";

import { isNodeLike } from "@azure/core-util";
import { BlobImmutabilityPolicyMode } from "./generatedModels";
import {
BlobDownloadHeaders,
BlobType,
Expand Down Expand Up @@ -500,7 +500,7 @@ export class BlobDownloadResponse implements BlobDownloadResponseParsed {
* @readonly
*/
public get readableStreamBody(): NodeJS.ReadableStream | undefined {
return isNode ? this.blobDownloadStream : undefined;
return isNodeLike ? this.blobDownloadStream : undefined;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions sdk/storage/storage-blob/src/BlobServiceClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT license.
import { TokenCredential, isTokenCredential } from "@azure/core-auth";
import { getDefaultProxySettings } from "@azure/core-rest-pipeline";
import { isNode } from "@azure/core-util";
import { isNodeLike } from "@azure/core-util";
import { AbortSignalLike } from "@azure/abort-controller";
import {
ServiceGetUserDelegationKeyHeaders,
Expand Down Expand Up @@ -360,7 +360,7 @@ export class BlobServiceClient extends StorageClient {
options = options || {};
const extractedCreds = extractConnectionStringParts(connectionString);
if (extractedCreds.kind === "AccountConnString") {
if (isNode) {
if (isNodeLike) {
const sharedKeyCredential = new StorageSharedKeyCredential(
extractedCreds.accountName!,
extractedCreds.accountKey,
Expand Down Expand Up @@ -451,7 +451,7 @@ export class BlobServiceClient extends StorageClient {
if (isPipelineLike(credentialOrPipeline)) {
pipeline = credentialOrPipeline;
} else if (
(isNode && credentialOrPipeline instanceof StorageSharedKeyCredential) ||
(isNodeLike && credentialOrPipeline instanceof StorageSharedKeyCredential) ||
credentialOrPipeline instanceof AnonymousCredential ||
isTokenCredential(credentialOrPipeline)
) {
Expand Down
27 changes: 13 additions & 14 deletions sdk/storage/storage-blob/src/Clients.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import {
TransferProgressEvent,
} from "@azure/core-rest-pipeline";
import { isTokenCredential, TokenCredential } from "@azure/core-auth";
import { isNode } from "@azure/core-util";
import { isNodeLike } from "@azure/core-util";
import { PollOperationState } from "@azure/core-lro";
import { randomUUID } from "@azure/core-util";
import { Readable } from "stream";

import { BlobDownloadResponse } from "./BlobDownloadResponse";
import { BlobQueryResponse } from "./BlobQueryResponse";
import { AnonymousCredential } from "./credentials/AnonymousCredential";
Expand Down Expand Up @@ -996,7 +995,7 @@ export class BlobClient extends StorageClient {
url = urlOrConnectionString;
pipeline = credentialOrPipelineOrContainerName;
} else if (
(isNode && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
(isNodeLike && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
credentialOrPipelineOrContainerName instanceof AnonymousCredential ||
isTokenCredential(credentialOrPipelineOrContainerName)
) {
Expand Down Expand Up @@ -1027,7 +1026,7 @@ export class BlobClient extends StorageClient {

const extractedCreds = extractConnectionStringParts(urlOrConnectionString);
if (extractedCreds.kind === "AccountConnString") {
if (isNode) {
if (isNodeLike) {
const sharedKeyCredential = new StorageSharedKeyCredential(
extractedCreds.accountName!,
extractedCreds.accountKey,
Expand Down Expand Up @@ -1210,7 +1209,7 @@ export class BlobClient extends StorageClient {
ifTags: options.conditions?.tagConditions,
},
requestOptions: {
onDownloadProgress: isNode ? undefined : options.onProgress, // for Node.js, progress is reported by RetriableReadableStream
onDownloadProgress: isNodeLike ? undefined : options.onProgress, // for Node.js, progress is reported by RetriableReadableStream
},
range: offset === 0 && !count ? undefined : rangeToString({ offset, count }),
rangeGetContentMD5: options.rangeGetContentMD5,
Expand All @@ -1228,7 +1227,7 @@ export class BlobClient extends StorageClient {
objectReplicationSourceProperties: parseObjectReplicationRecord(res.objectReplicationRules),
};
// Return browser response immediately
if (!isNode) {
if (!isNodeLike) {
return wrappedRes;
}

Expand Down Expand Up @@ -2533,7 +2532,7 @@ export class AppendBlobClient extends BlobClient {
url = urlOrConnectionString;
pipeline = credentialOrPipelineOrContainerName;
} else if (
(isNode && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
(isNodeLike && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
credentialOrPipelineOrContainerName instanceof AnonymousCredential ||
isTokenCredential(credentialOrPipelineOrContainerName)
) {
Expand Down Expand Up @@ -2561,7 +2560,7 @@ export class AppendBlobClient extends BlobClient {

const extractedCreds = extractConnectionStringParts(urlOrConnectionString);
if (extractedCreds.kind === "AccountConnString") {
if (isNode) {
if (isNodeLike) {
const sharedKeyCredential = new StorageSharedKeyCredential(
extractedCreds.accountName!,
extractedCreds.accountKey,
Expand Down Expand Up @@ -3517,7 +3516,7 @@ export class BlockBlobClient extends BlobClient {
url = urlOrConnectionString;
pipeline = credentialOrPipelineOrContainerName;
} else if (
(isNode && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
(isNodeLike && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
credentialOrPipelineOrContainerName instanceof AnonymousCredential ||
isTokenCredential(credentialOrPipelineOrContainerName)
) {
Expand Down Expand Up @@ -3548,7 +3547,7 @@ export class BlockBlobClient extends BlobClient {

const extractedCreds = extractConnectionStringParts(urlOrConnectionString);
if (extractedCreds.kind === "AccountConnString") {
if (isNode) {
if (isNodeLike) {
const sharedKeyCredential = new StorageSharedKeyCredential(
extractedCreds.accountName!,
extractedCreds.accountKey,
Expand Down Expand Up @@ -3642,7 +3641,7 @@ export class BlockBlobClient extends BlobClient {
options: BlockBlobQueryOptions = {},
): Promise<BlobDownloadResponseModel> {
ensureCpkIfSpecified(options.customerProvidedKey, this.isHttps);
if (!isNode) {
if (!isNodeLike) {
throw new Error("This operation currently is only supported in Node.js.");
}

Expand Down Expand Up @@ -3994,7 +3993,7 @@ export class BlockBlobClient extends BlobClient {
options: BlockBlobParallelUploadOptions = {},
): Promise<BlobUploadCommonResponse> {
return tracingClient.withSpan("BlockBlobClient-uploadData", options, async (updatedOptions) => {
if (isNode) {
if (isNodeLike) {
let buffer: Buffer;
if (data instanceof Buffer) {
buffer = data;
Expand Down Expand Up @@ -4783,7 +4782,7 @@ export class PageBlobClient extends BlobClient {
url = urlOrConnectionString;
pipeline = credentialOrPipelineOrContainerName;
} else if (
(isNode && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
(isNodeLike && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
credentialOrPipelineOrContainerName instanceof AnonymousCredential ||
isTokenCredential(credentialOrPipelineOrContainerName)
) {
Expand Down Expand Up @@ -4811,7 +4810,7 @@ export class PageBlobClient extends BlobClient {

const extractedCreds = extractConnectionStringParts(urlOrConnectionString);
if (extractedCreds.kind === "AccountConnString") {
if (isNode) {
if (isNodeLike) {
const sharedKeyCredential = new StorageSharedKeyCredential(
extractedCreds.accountName!,
extractedCreds.accountKey,
Expand Down
6 changes: 3 additions & 3 deletions sdk/storage/storage-blob/src/ContainerClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
getDefaultProxySettings,
RequestBodyType as HttpRequestBody,
} from "@azure/core-rest-pipeline";
import { isNode } from "@azure/core-util";
import { isNodeLike } from "@azure/core-util";
import { TokenCredential, isTokenCredential } from "@azure/core-auth";
import { PagedAsyncIterableIterator, PageSettings } from "@azure/core-paging";
import { AnonymousCredential } from "./credentials/AnonymousCredential";
Expand Down Expand Up @@ -659,7 +659,7 @@ export class ContainerClient extends StorageClient {
url = urlOrConnectionString;
pipeline = credentialOrPipelineOrContainerName;
} else if (
(isNode && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
(isNodeLike && credentialOrPipelineOrContainerName instanceof StorageSharedKeyCredential) ||
credentialOrPipelineOrContainerName instanceof AnonymousCredential ||
isTokenCredential(credentialOrPipelineOrContainerName)
) {
Expand All @@ -683,7 +683,7 @@ export class ContainerClient extends StorageClient {

const extractedCreds = extractConnectionStringParts(urlOrConnectionString);
if (extractedCreds.kind === "AccountConnString") {
if (isNode) {
if (isNodeLike) {
const sharedKeyCredential = new StorageSharedKeyCredential(
extractedCreds.accountName!,
extractedCreds.accountKey,
Expand Down
5 changes: 2 additions & 3 deletions sdk/storage/storage-blob/src/policies/StorageBrowserPolicy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import {
CompatResponse as HttpOperationResponse,
} from "@azure/core-http-compat";
import { BaseRequestPolicy } from "./RequestPolicy";
import { isNode } from "@azure/core-util";

import { isNodeLike } from "@azure/core-util";
import { HeaderConstants, URLConstants } from "../utils/constants";
import { setURLParameter } from "../utils/utils.common";

Expand Down Expand Up @@ -42,7 +41,7 @@ export class StorageBrowserPolicy extends BaseRequestPolicy {
* @param request -
*/
public async sendRequest(request: WebResource): Promise<HttpOperationResponse> {
if (isNode) {
if (isNodeLike) {
return this._nextPolicy.sendRequest(request);
}

Expand Down
4 changes: 2 additions & 2 deletions sdk/storage/storage-blob/src/policies/StorageBrowserPolicyV2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
SendRequest,
PipelinePolicy,
} from "@azure/core-rest-pipeline";
import { isNode } from "@azure/core-util";
import { isNodeLike } from "@azure/core-util";
import { HeaderConstants, URLConstants } from "../utils/constants";
import { setURLParameter } from "../utils/utils.common";

Expand All @@ -24,7 +24,7 @@ export function storageBrowserPolicy(): PipelinePolicy {
return {
name: storageBrowserPolicyName,
async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> {
if (isNode) {
if (isNodeLike) {
return next(request);
}

Expand Down
7 changes: 3 additions & 4 deletions sdk/storage/storage-blob/src/utils/utils.common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
import { AbortSignalLike } from "@azure/abort-controller";
import { TokenCredential } from "@azure/core-auth";
import { HttpHeaders, createHttpHeaders } from "@azure/core-rest-pipeline";
import { isNode } from "@azure/core-util";

import { isNodeLike } from "@azure/core-util";
import {
BlobQueryArrowConfiguration,
BlobQueryCsvTextConfiguration,
Expand Down Expand Up @@ -461,7 +460,7 @@ export function truncatedISO8061Date(date: Date, withMilliseconds: boolean = tru
* @param content -
*/
export function base64encode(content: string): string {
return !isNode ? btoa(content) : Buffer.from(content).toString("base64");
return !isNodeLike ? btoa(content) : Buffer.from(content).toString("base64");
}

/**
Expand All @@ -470,7 +469,7 @@ export function base64encode(content: string): string {
* @param encodedString -
*/
export function base64decode(encodedString: string): string {
return !isNode ? atob(encodedString) : Buffer.from(encodedString, "base64").toString();
return !isNodeLike ? atob(encodedString) : Buffer.from(encodedString, "base64").toString();
}

/**
Expand Down
Loading