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

[rollup] - Fix bug in circular dependency warning suppression #19012

Merged
merged 25 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 11 additions & 10 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"]
Copy link
Member

Choose a reason for hiding this comment

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

I think the definition of openTelemetryCommonJs should be deleted too if it is no longer needed.

I wonder who uses events.

Copy link
Contributor

Choose a reason for hiding this comment

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

events started out in the template package and my guess is that it got copied around :)

Copy link
Member Author

Choose a reason for hiding this comment

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

We're close to being able to remove this crud - I just know that every rollup file imports this method and uses it, so rather than increasing the PR size I opted to leave the definiton. But I will log a separate issue for me to remove it (here and everywhere that has it)

}
}),
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"));

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
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
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
Loading