[core-rest-pipeline] Add conditional exports (#22804)

- Add conditional exports for core-rest-pipeline - Add .js extension to imported source module name - Bump dev dependency Mocha version to v10 - Add `ts-node/esm` loader in mocha config - Update npm scripts - Update linter rule to allow dist/index.cjs - Add a copy of our typing files for cjs - Bump minor version ### Packages impacted by this PR `@azure/core-rest-pipeline` ### Issues associated with this PR #22172
Azure · Oct 20, 2022 · 02f6ad6 · 02f6ad6
1 parent bfb1bc2
commit 02f6ad6
Show file tree

Hide file tree

Showing 78 changed files with 421 additions and 259 deletions.
diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml
diff --git a/common/tools/dev-tool/src/commands/run/bundle.ts b/common/tools/dev-tool/src/commands/run/bundle.ts
@@ -70,9 +70,12 @@ export default leafCommand(commandInfo, async (options) => {
 
     try {
       const bundle = await rollup.rollup(baseConfig);
-
+      const cjsFilename = info.packageJson.main;
+      if (!cjsFilename) {
+        throw new Error("Expecting valid main entry");
+      }
       await bundle.write({
-        file: "dist/index.js",
+        file: cjsFilename,
         format: "cjs",
         sourcemap: true,
         exports: "named",

diff --git a/common/tools/dev-tool/src/commands/run/testNodeJSInput.ts b/common/tools/dev-tool/src/commands/run/testNodeJSInput.ts
@@ -12,8 +12,9 @@ export const commandInfo = makeCommandInfo(
 export default leafCommand(commandInfo, async (options) => {
   const defaultMochaArgs =
     "-r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --full-trace";
-  const updatedArgs = options["--"]?.map(opt =>
-    opt.includes("**") && !opt.startsWith("'") && !opt.startsWith('"') ? `"${opt}"` : opt)
+  const updatedArgs = options["--"]?.map((opt) =>
+    opt.includes("**") && !opt.startsWith("'") && !opt.startsWith('"') ? `"${opt}"` : opt
+  );
   const mochaArgs = updatedArgs?.length
     ? updatedArgs?.join(" ")
     : '--timeout 5000000 "dist-esm/test/{,!(browser)/**/}/*.spec.js"';

diff --git a/common/tools/dev-tool/src/commands/run/testNodeTSInput.ts b/common/tools/dev-tool/src/commands/run/testNodeTSInput.ts
@@ -12,8 +12,9 @@ export const commandInfo = makeCommandInfo(
 export default leafCommand(commandInfo, async (options) => {
   const defaultMochaArgs =
     "-r esm -r ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --full-trace";
-  const updatedArgs = options["--"]?.map(opt =>
-    opt.includes("**") && !opt.startsWith("'") && !opt.startsWith('"') ? `"${opt}"` : opt)
+  const updatedArgs = options["--"]?.map((opt) =>
+    opt.includes("**") && !opt.startsWith("'") && !opt.startsWith('"') ? `"${opt}"` : opt
+  );
   const mochaArgs = updatedArgs?.length
     ? updatedArgs?.join(" ")
     : '--timeout 1200000 --exclude "test/**/browser/*.spec.ts" "test/**/*.spec.ts"';

diff --git a/common/tools/dev-tool/src/config/rollup.base.config.ts b/common/tools/dev-tool/src/config/rollup.base.config.ts
@@ -55,7 +55,7 @@ export function sourcemapsExtra() {
         },
       });
 
-      return load instanceof Function ? load.call(shim, id): load.handler.call(shim, id);
+      return load instanceof Function ? load.call(shim, id) : load.handler.call(shim, id);
     },
   });
 }

diff --git a/common/tools/dev-tool/src/util/samples/generation.ts b/common/tools/dev-tool/src/util/samples/generation.ts
@@ -321,16 +321,18 @@ export async function makeSamplesFactory(
    */
   function postProcess(moduleText: string | Buffer): string {
     const content = Buffer.isBuffer(moduleText) ? moduleText.toString("utf8") : moduleText;
-    return content
-      .replace(new RegExp(`^\\s*\\*\\s*@${AZSDK_META_TAG_PREFIX}.*\n`, "gm"), "")
-      // We also need to clean up extra blank lines that might be left behind by
-      // removing azsdk tags. These regular expressions are extremely frustrating
-      // because they deal almost exclusively in the literal "/" and "*" characters.
-      .replace(/(\s+\*)+\//s, "\n */")
-      // Clean up blank lines at the beginning
-      .replace(/\/\*\*(\s+\*)*/s, `/**\n *`)
-      // Finally remove empty doc comments.
-      .replace(/\s*\/\*\*(\s+\*)*\/\s*/s, "\n\n");
+    return (
+      content
+        .replace(new RegExp(`^\\s*\\*\\s*@${AZSDK_META_TAG_PREFIX}.*\n`, "gm"), "")
+        // We also need to clean up extra blank lines that might be left behind by
+        // removing azsdk tags. These regular expressions are extremely frustrating
+        // because they deal almost exclusively in the literal "/" and "*" characters.
+        .replace(/(\s+\*)+\//s, "\n */")
+        // Clean up blank lines at the beginning
+        .replace(/\/\*\*(\s+\*)*/s, `/**\n *`)
+        // Finally remove empty doc comments.
+        .replace(/\s*\/\*\*(\s+\*)*\/\s*/s, "\n\n")
+    );
   }
 
   // We use a tempdir at the outer layer to avoid creating dirty trees

diff --git a/common/tools/dev-tool/src/util/testProxyUtils.ts b/common/tools/dev-tool/src/util/testProxyUtils.ts
@@ -14,7 +14,11 @@ const log = createPrinter("test-proxy");
 const CONTAINER_NAME = "js-azsdk-test-proxy";
 
 export async function startProxyTool(): Promise<void> {
-  log.info(`Attempting to start test proxy at http://localhost:${process.env.TEST_PROXY_HTTP_PORT ?? 5000} & https://localhost:${process.env.TEST_PROXY_HTTPS_PORT ?? 5001}.\n`);
+  log.info(
+    `Attempting to start test proxy at http://localhost:${
+      process.env.TEST_PROXY_HTTP_PORT ?? 5000
+    } & https://localhost:${process.env.TEST_PROXY_HTTPS_PORT ?? 5001}.\n`
+  );
 
   const subprocess = spawn(await getDockerRunCommand(), [], {
     shell: true,
@@ -59,13 +63,24 @@ async function getDockerRunCommand() {
   const testProxyRecordingsLocation = "/srv/testproxy";
   const allowLocalhostAccess = "--add-host host.docker.internal:host-gateway";
   const imageToLoad = `azsdkengsys.azurecr.io/engsys/testproxy-lin:${await getImageTag()}`;
-  return `docker run --rm --name ${CONTAINER_NAME} -v ${repoRoot}:${testProxyRecordingsLocation} -p ${process.env.TEST_PROXY_HTTPS_PORT ?? 5001}:5001 -p ${process.env.TEST_PROXY_HTTP_PORT ?? 5000}:5000 ${allowLocalhostAccess} ${imageToLoad}`;
+  return `docker run --rm --name ${CONTAINER_NAME} -v ${repoRoot}:${testProxyRecordingsLocation} -p ${
+    process.env.TEST_PROXY_HTTPS_PORT ?? 5001
+  }:5001 -p ${
+    process.env.TEST_PROXY_HTTP_PORT ?? 5000
+  }:5000 ${allowLocalhostAccess} ${imageToLoad}`;
 }
 
 export async function isProxyToolActive(): Promise<boolean> {
   try {
-    await makeRequest(`http://localhost:${process.env.TEST_PROXY_HTTP_PORT ?? 5000}/info/available`, {});
-    log.info(`Proxy tool seems to be active at http://localhost:${process.env.TEST_PROXY_HTTP_PORT ?? 5000}\n`);
+    await makeRequest(
+      `http://localhost:${process.env.TEST_PROXY_HTTP_PORT ?? 5000}/info/available`,
+      {}
+    );
+    log.info(
+      `Proxy tool seems to be active at http://localhost:${
+        process.env.TEST_PROXY_HTTP_PORT ?? 5000
+      }\n`
+    );
     return true;
   } catch (error: any) {
     return false;

diff --git a/common/tools/dev-tool/src/util/testUtils.ts b/common/tools/dev-tool/src/util/testUtils.ts
@@ -1,5 +1,9 @@
 import { isProxyToolActive } from "./testProxyUtils";
-import concurrently, { Command as ConcurrentlyCommand, ConcurrentlyCommandInput, ConcurrentlyOptions } from "concurrently";
+import concurrently, {
+  Command as ConcurrentlyCommand,
+  ConcurrentlyCommandInput,
+  ConcurrentlyOptions,
+} from "concurrently";
 import { createPrinter } from "./printer";
 
 const log = createPrinter("preparing-proxy-tool");
@@ -15,7 +19,9 @@ async function shouldRunProxyTool(): Promise<boolean> {
       // No need to run a new one if it is already active
       // Especially, CI uses this path
       log.info(
-        `Proxy tool seems to be active, not attempting to start the test proxy at http://localhost:${process.env.TEST_PROXY_HTTP_PORT ?? 5000} & https://localhost:${process.env.TEST_PROXY_HTTPS_PORT ?? 5001}.\n`
+        `Proxy tool seems to be active, not attempting to start the test proxy at http://localhost:${
+          process.env.TEST_PROXY_HTTP_PORT ?? 5000
+        } & https://localhost:${process.env.TEST_PROXY_HTTPS_PORT ?? 5001}.\n`
       );
     }
     return !isActive;

diff --git a/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-main-is-cjs.md b/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-main-is-cjs.md
@@ -1,6 +1,6 @@
 # ts-package-json-main-is-cjs
 
-Requires `main` in `package.json` to be point to a CommonJS or UMD module. In this case, its assumed that it points to `"dist/index.js"`.
+Requires `main` in `package.json` to be point to a CommonJS or UMD module. In this case, its assumed that it points to `"dist/index.js"` or `"dist/index.cjs"`.
 
 This rule is fixable using the `--fix` option.
 
@@ -14,6 +14,14 @@ This rule is fixable using the `--fix` option.
 }
 ```
 
+### Good
+
+```json
+{
+  "main": "dist/index.cjs"
+}
+```
+
 ### Bad
 
 ```json

diff --git a/common/tools/eslint-plugin-azure-sdk/src/rules/ts-package-json-main-is-cjs.ts b/common/tools/eslint-plugin-azure-sdk/src/rules/ts-package-json-main-is-cjs.ts
@@ -45,10 +45,10 @@ export = {
             const nodeValue = node.value as Literal;
             const main = nodeValue.value as string;
 
-            if (!/^(\.\/)?dist\/index\.js$/.test(main)) {
+            if (!/^(\.\/)?dist\/index\.(c)?js$/.test(main)) {
               context.report({
                 node: nodeValue,
-                message: `main is set to ${main} when it should be set to dist/index.js`,
+                message: `main is set to ${main} when it should be set to dist/index.js or dist/index.cjs`,
                 fix: (fixer: Rule.RuleFixer): Rule.Fix =>
                   fixer.replaceText(nodeValue, `"dist/index.js"`),
               });

diff --git a/common/tools/eslint-plugin-azure-sdk/tests/rules/ts-package-json-main-is-cjs.ts b/common/tools/eslint-plugin-azure-sdk/tests/rules/ts-package-json-main-is-cjs.ts
@@ -263,6 +263,16 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       code: '{"main": "./dist/index.js"}',
       filename: "package.json",
     },
+    {
+      // correct format #1
+      code: '{"main": "dist/index.cjs"}',
+      filename: "package.json",
+    },
+    {
+      // correct format #2
+      code: '{"main": "./dist/index.cjs"}',
+      filename: "package.json",
+    },
     {
       // a full example package.json (taken from https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/eventhub/event-hubs/package.json with "scripts" removed for testing purposes)
       code: examplePackageGood,
@@ -300,7 +310,8 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       filename: "package.json",
       errors: [
         {
-          message: "main is set to dist//index.js when it should be set to dist/index.js",
+          message:
+            "main is set to dist//index.js when it should be set to dist/index.js or dist/index.cjs",
         },
       ],
       output: '{"main": "dist/index.js"}',
@@ -310,7 +321,8 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       filename: "package.json",
       errors: [
         {
-          message: "main is set to .dist/index.js when it should be set to dist/index.js",
+          message:
+            "main is set to .dist/index.js when it should be set to dist/index.js or dist/index.cjs",
         },
       ],
       output: '{"main": "dist/index.js"}',
@@ -320,7 +332,8 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       filename: "package.json",
       errors: [
         {
-          message: "main is set to /dist/index.js when it should be set to dist/index.js",
+          message:
+            "main is set to /dist/index.js when it should be set to dist/index.js or dist/index.cjs",
         },
       ],
       output: '{"main": "dist/index.js"}',
@@ -331,7 +344,7 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       filename: "package.json",
       errors: [
         {
-          message: "main is set to dist when it should be set to dist/index.js",
+          message: "main is set to dist when it should be set to dist/index.js or dist/index.cjs",
         },
       ],
       output: '{"main": "dist/index.js"}',
@@ -341,7 +354,8 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       filename: "package.json",
       errors: [
         {
-          message: "main is set to index.js when it should be set to dist/index.js",
+          message:
+            "main is set to index.js when it should be set to dist/index.js or dist/index.cjs",
         },
       ],
       output: '{"main": "dist/index.js"}',
@@ -351,7 +365,8 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       filename: "package.json",
       errors: [
         {
-          message: "main is set to dist/src/index.js when it should be set to dist/index.js",
+          message:
+            "main is set to dist/src/index.js when it should be set to dist/index.js or dist/index.cjs",
         },
       ],
       output: '{"main": "dist/index.js"}',
@@ -362,7 +377,8 @@ ruleTester.run("ts-package-json-main-is-cjs", rule, {
       filename: "package.json",
       errors: [
         {
-          message: "main is set to index.js when it should be set to dist/index.js",
+          message:
+            "main is set to index.js when it should be set to dist/index.js or dist/index.cjs",
         },
       ],
       output: examplePackageGood,

diff --git a/sdk/core/core-rest-pipeline/CHANGELOG.md b/sdk/core/core-rest-pipeline/CHANGELOG.md
@@ -1,9 +1,11 @@
 # Release History
 
-## 1.9.3 (Unreleased)
+## 1.10.0 (Unreleased)
 
 ### Features Added
 
+- Added conditional exports for CommonJS and ESM. [#22804](https://github.com/Azure/azure-sdk-for-js/pull/22804)
+
 ### Breaking Changes
 
 ### Bugs Fixed

diff --git a/sdk/core/core-rest-pipeline/core-rest-pipeline.d.cts b/sdk/core/core-rest-pipeline/core-rest-pipeline.d.cts
@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+declare global {
+  interface FormData {}
+  interface Blob {}
+  interface File {}
+  interface ReadableStream<R = any> {}
+  interface TransformStream<I = any, O = any>  {}
+}
+
+export * from "./types/latest/core-rest-pipeline";
-Original file line number
+Diff line change
@@ Expand Up / @@ -55,7 +55,7 @@ export function sourcemapsExtra() { @@
             },
           });
-          return load instanceof Function ? load.call(shim, id): load.handler.call(shim, id);
+          return load instanceof Function ? load.call(shim, id) : load.handler.call(shim, id);
         },
       });
     }
@@ Expand Down @@