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-rest-pipeline] Add conditional exports #22804

Merged
merged 19 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ee27d04
Conditional export for core-rest-pipeline
jeremymeng Jun 13, 2022
1c0c4fb
Add an option to dev-tool bundle command for .cjs output extension
jeremymeng Aug 1, 2022
db7e7ef
import "*" => import "*.js"
jeremymeng Aug 1, 2022
aa393d5
bump mocha version to ^10.0.0
jeremymeng Aug 2, 2022
31632f4
- import * as => import
jeremymeng Aug 4, 2022
db5ed1f
Merge remote-tracking branch 'up/main' into core/cond-export
jeremymeng Aug 4, 2022
d9fe9ee
- Fix linter rule to allow `"main": "dist/index.cjs"`
jeremymeng Aug 4, 2022
d874bc1
revert back to awesome example.com
jeremymeng Aug 4, 2022
65524e6
Fix types export
jeremymeng Aug 4, 2022
76a44b5
Merge remote-tracking branch 'up/main' into core/cond-export
jeremymeng Aug 4, 2022
5231081
Bump minor version and update CHANGELOG
jeremymeng Aug 4, 2022
279160c
Merge remote-tracking branch 'up/main' into core/cond-export
jeremymeng Aug 5, 2022
00d9c25
Merge remote-tracking branch 'up/main' into core/cond-export
jeremymeng Aug 29, 2022
6bcb49c
Merge remote-tracking branch 'up/main' into core/cond-export
jeremymeng Oct 5, 2022
10f3fa6
Add copy of our types shim for CommonJS conditional export
jeremymeng Oct 6, 2022
5bb5b13
remove `output-cjs-ext` option
jeremymeng Oct 17, 2022
ccc1365
Merge remote-tracking branch 'upstream/main' into core/cond-export
jeremymeng Oct 17, 2022
57705cf
delete usage of removed dev-tool bundle option
jeremymeng Oct 20, 2022
df32ab1
Merge remote-tracking branch 'up/main' into core/cond-export
jeremymeng Oct 20, 2022
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
54 changes: 47 additions & 7 deletions common/config/rush/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions common/tools/dev-tool/src/commands/run/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions common/tools/dev-tool/src/commands/run/testNodeJSInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"';
Expand Down
5 changes: 3 additions & 2 deletions common/tools/dev-tool/src/commands/run/testNodeTSInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"';
Expand Down
2 changes: 1 addition & 1 deletion common/tools/dev-tool/src/config/rollup.base.config.ts
Original file line number Diff 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
22 changes: 12 additions & 10 deletions common/tools/dev-tool/src/util/samples/generation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 19 additions & 4 deletions common/tools/dev-tool/src/util/testProxyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 8 additions & 2 deletions common/tools/dev-tool/src/util/testUtils.ts
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

Expand All @@ -14,6 +14,14 @@ This rule is fixable using the `--fix` option.
}
```

### Good

```json
{
"main": "dist/index.cjs"
}
```

### Bad

```json
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"}',
Expand All @@ -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"}',
Expand All @@ -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"}',
Expand All @@ -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"}',
Expand All @@ -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"}',
Expand All @@ -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"}',
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion sdk/core/core-rest-pipeline/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
12 changes: 12 additions & 0 deletions sdk/core/core-rest-pipeline/core-rest-pipeline.d.cts
Original file line number Diff line number Diff line change
@@ -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";
Loading