Skip to content

Commit

Permalink
[rollup] - Fix bug in circular dependency warning suppression (#19012)
Browse files Browse the repository at this point in the history
## What

- Fix incorrect logic when suppressing chai's circular dependency warnings
- Move to the common dev-tool configuration where possible

## Why

This is a longstanding issue that we have, where an incorrect logic was copy-pasted to other places. I figured while cleaning this up that any package I touch can just convert over to the shared dev-tool configuration. Where I was unable
to do that, I just fixed this bug to avoid too many changes in one PR.

Fixes #14292
Resolves #17818 
Resolves #17816 
Resolves #17815 
Resolves #17814 
Resolves #17813 
Resolves #17810
  • Loading branch information
maorleger authored Dec 9, 2021
1 parent d810a31 commit 3c97db3
Show file tree
Hide file tree
Showing 53 changed files with 66 additions and 1,888 deletions.
19 changes: 10 additions & 9 deletions common/config/rush/pnpm-lock.yaml

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

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 @@ -125,7 +125,7 @@ export function makeBrowserTestConfig(): RollupOptions {
// Chai's strange internal architecture makes it impossible to statically
// analyze its exports.
chai: ["version", "use", "util", "config", "expect", "should", "assert"],
...openTelemetryCommonJs()
events: ["EventEmitter"]
}
}),
json(),
Expand Down
15 changes: 3 additions & 12 deletions sdk/anomalydetector/ai-anomaly-detector/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
"build:browser": "tsc -p . && cross-env ONLY_BROWSER=true rollup -c 2>&1",
"build:node": "tsc -p . && cross-env ONLY_NODE=true rollup -c 2>&1",
"build:samples": "echo Obsolete.",
"build:test": "tsc -p . && rollup -c rollup.test.config.js 2>&1",
"build:test": "tsc -p . && rollup -c 2>&1",
"build": "npm run clean && tsc -p . && rollup -c 2>&1 && api-extractor run --local",
"check-format": "prettier --list-different --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"clean": "rimraf dist dist-* temp types *.tgz *.log",
"execute:samples": "npm run build:samples && dev-tool samples run dist-samples/javascript dist-samples/typescript/dist/dist-samples/typescript/src/",
"extract-api": "tsc -p . && api-extractor run --local",
"format": "prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "karma start --single-run",
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 5000000 --full-trace dist-esm/test/**/*.spec.js",
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 5000000 --full-trace \"dist-esm/test/{,!(browser)/**/}*.spec.js\"",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"lint:fix": "eslint package.json api-extractor.json src test --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json api-extractor.json src test --ext .ts",
Expand All @@ -31,7 +31,7 @@
"test:node": "npm run build:test && npm run unit-test:node && npm run integration-test:node",
"test": "npm run build:test && npm run unit-test && npm run integration-test",
"unit-test:browser": "karma start --single-run",
"unit-test:node": "mocha --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace dist-test/index.node.js",
"unit-test:node": "mocha -r esm --require ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"test/{,!(browser)/**/}*.spec.ts\"",
"unit-test": "npm run unit-test:node && npm run unit-test:browser",
"docs": "typedoc --excludePrivate --excludeNotExported --excludeExternals --stripInternal --mode file --out ./dist/docs ./src"
},
Expand Down Expand Up @@ -73,11 +73,6 @@
"@azure/identity": "^2.0.1",
"@azure/eslint-plugin-azure-sdk": "^3.0.0",
"@microsoft/api-extractor": "^7.18.11",
"@rollup/plugin-commonjs": "11.0.2",
"@rollup/plugin-json": "^4.0.0",
"@rollup/plugin-multi-entry": "^3.0.0",
"@rollup/plugin-node-resolve": "^8.0.0",
"@rollup/plugin-replace": "^2.2.0",
"@types/chai": "^4.1.6",
"@types/mocha": "^7.0.2",
"@types/node": "^12.0.0",
Expand All @@ -104,10 +99,6 @@
"prettier": "^1.16.4",
"rimraf": "^3.0.0",
"rollup": "^1.16.3",
"rollup-plugin-shim": "^1.0.0",
"rollup-plugin-sourcemaps": "^0.4.2",
"rollup-plugin-terser": "^5.1.1",
"rollup-plugin-visualizer": "^4.0.4",
"typescript": "~4.2.0",
"util": "^0.12.1",
"@azure-tools/test-recorder": "^1.0.0",
Expand Down
137 changes: 0 additions & 137 deletions sdk/anomalydetector/ai-anomaly-detector/rollup.base.config.js

This file was deleted.

14 changes: 2 additions & 12 deletions sdk/anomalydetector/ai-anomaly-detector/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
import * as base from "./rollup.base.config";
import { makeConfig } from "@azure/dev-tool/shared-config/rollup";

const inputs = [];

if (!process.env.ONLY_BROWSER) {
inputs.push(base.nodeConfig());
}

if (!process.env.ONLY_NODE) {
inputs.push(base.browserConfig());
}

export default inputs;
export default makeConfig(require("./package.json"));
3 changes: 0 additions & 3 deletions sdk/anomalydetector/ai-anomaly-detector/rollup.test.config.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@

import { Context } from "mocha";

import * as dotenv from "dotenv";
import { env, Recorder, record, RecorderEnvironmentSetup } from "@azure-tools/test-recorder";

import { ClientSecretCredential } from "@azure/identity";
import { AnomalyDetectorClient } from "../../src/AnomalyDetectorClient";
import { AzureKeyCredential } from "@azure/core-auth";

dotenv.config();

export interface RecordedRecognizerClient {
client: AnomalyDetectorClient;
recorder: Recorder;
Expand Down
5 changes: 1 addition & 4 deletions sdk/appconfiguration/app-configuration/rollup.base.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ const ignoreKnownWarnings = (warning) => {
return;
}

if (
warning.code === "CIRCULAR_DEPENDENCY" &&
warning.importer.indexOf(path.normalize("node_modules/chai/lib") === 0)
) {
if (warning.code === "CIRCULAR_DEPENDENCY" && warning.importer.includes("node_modules/chai")) {
// Chai contains circular references, but they are not fatal and can be ignored.
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ import {
} from "@azure-tools/test-recorder";
import * as assert from "assert";

// allow loading from a .env file as an alternative to defining the variable
// in the environment
import * as dotenv from "dotenv";

import { DefaultAzureCredential, TokenCredential } from "@azure/identity";
dotenv.config();

let connectionStringNotPresentWarning = false;
let tokenCredentialsNotPresentWarning = false;
Expand Down
2 changes: 1 addition & 1 deletion sdk/communication/communication-chat/rollup.base.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export function browserConfig(test = false) {
baseConfig.onwarn = (warning) => {
if (
warning.code === "CIRCULAR_DEPENDENCY" &&
warning.importer.indexOf(path.normalize("node_modules/chai/lib") === 0)
warning.importer.includes("node_modules/chai")
) {
// Chai contains circular references, but they are not fatal and can be ignored.
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export function browserConfig(test = false) {
baseConfig.onwarn = (warning) => {
if (
warning.code === "CIRCULAR_DEPENDENCY" &&
warning.importer.indexOf(path.normalize("node_modules/chai/lib") === 0)
warning.importer.includes("node_modules/chai")
) {
// Chai contains circular references, but they are not fatal and can be ignored.
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function browserConfig(test = false) {
baseConfig.onwarn = (warning) => {
if (
warning.code === "CIRCULAR_DEPENDENCY" &&
warning.importer.indexOf(path.normalize("node_modules/chai/lib") === 0)
warning.importer.includes("node_modules/chai")
) {
// Chai contains circular references, but they are not fatal and can be ignored.
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function browserConfig(test = false) {
baseConfig.onwarn = (warning) => {
if (
warning.code === "CIRCULAR_DEPENDENCY" &&
warning.importer.indexOf(path.normalize("node_modules/chai/lib") === 0)
warning.importer.includes("node_modules/chai")
) {
// Chai contains circular references, but they are not fatal and can be ignored.
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export function browserConfig(test = false) {
baseConfig.onwarn = (warning) => {
if (
warning.code === "CIRCULAR_DEPENDENCY" &&
warning.importer.indexOf(path.normalize("node_modules/chai/lib") === 0)
warning.importer.includes("node_modules/chai")
) {
// Chai contains circular references, but they are not fatal and can be ignored.
return;
Expand Down
Loading

0 comments on commit 3c97db3

Please sign in to comment.