Circular dependency

[jonathanblancor]

@azure/identity is throwing a Circular dependency warning. I have placed the text warning and picture.

I am importing @azure libraries as: import * as azureIdentity from '@azure/identity'; import * as appConfig from '@azure/app-configuration';

Warning: Circular dependency: node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\inode_modules\@opentelemetry\api\build\src\internal\global-utils.js -> node_modules\@azure\identity\node_modules\@opentelemetr Circular dependency: node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\inode_modules\@opentelemetry\api\build\src\internal\global-utils.js -> C:\Users\JBlanco\Documents\MFE - Product\node_modules\\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js Circular dependency: node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\tp\node_modules\@opentelemetry\api\build\src\internal\global-utils.js -> node_modules\@azure\core-http\node_modules\@opentele Circular dependency: node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\tp\node_modules\@opentelemetry\api\build\src\internal\global-utils.js -> C:\Users\JBlanco\Documents\MFE - Product\node_modulules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js

Any other information needed please let me know.

[sadasant]

Hello @jonathanblancor ! I’m Daniel. I’ll be doing my best to help you.

What version of @azure/identity and @azure/app-configuration are you using? Please make sure to install the latest versions available. For the purposes of a quick test, set the versions for both packages on your package.json as latest, then remove the node_modules, remove the package-lock.json, and install your dependencies. Is the error still there?

I won’t have time to investigate today, but I’ll take a look tomorrow for sure.

[jonathanblancor]

This is what I have: "@azure/app-configuration": "^1.1.1", "@azure/identity": "^1.3.0"

Aren't these the latest ones?

[sadasant]

@jonathanblancor Hello Jonathan,

Those are in fact the latest versions available.

We know of a certain case in which a similar problem appears, but we’ve only seen this while bundling applications, not while installing nor compiling. Would you mind giving us a concrete set of steps to reproduce this problem? Something we can reproduce on our own.

Thank you, your time is appreciated.

[jonathanblancor]

All I did was follow the steps in @azure/app-configuration and @azure/identity.

My package.json looks like:

{
  "name": "MFE-Product",
  "description": "MFE-Product",
  "version": "0.0.1",
  "scripts": {
    "dev": "sapper dev",
    "build": "sapper build --legacy",
    "export": "sapper export --legacy",
    "start": "node __sapper__/build",
    "test": "jest"
  },
  "dependencies": {
    "@azure/app-configuration": "^1.1.1",
    "@azure/identity": "^1.3.0",
    "@rollup/plugin-json": "^4.1.0",
    "applicationinsights": "^2.0.0",
    "ashcomm-css-purge": "0.0.3",
    "compression": "^1.7.1",
    "node-sass": "^5.0.0",
    "polka": "next",
    "rollup-plugin-svg": "^2.0.0",
    "sirv": "^1.0.0",
    "svelte-i18n": "^3.3.9"
  },
  "devDependencies": {
    "@babel/core": "^7.14.2",
    "@babel/plugin-syntax-dynamic-import": "^7.0.0",
    "@babel/plugin-transform-runtime": "^7.12.13",
    "@babel/preset-env": "^7.14.2",
    "@babel/runtime": "^7.12.13",
    "@rollup/plugin-babel": "^5.0.0",
    "@rollup/plugin-commonjs": "^14.0.0",
    "@rollup/plugin-node-resolve": "^8.0.0",
    "@rollup/plugin-replace": "^2.2.0",
    "@rollup/plugin-url": "^5.0.0",
    "@testing-library/jest-dom": "^5.12.0",
    "@testing-library/svelte": "^3.0.3",
    "ashcomm-core-svelte": "latest",
    "babel-jest": "^26.6.3",
    "css-purge": "^3.1.8",
    "jest": "^26.6.3",
    "rollup": "^2.7.6",
    "rollup-plugin-svelte": "^7.0.0",
    "rollup-plugin-terser": "^7.0.0",
    "sapper": "^0.28.0",
    "sass": "^1.32.7",
    "svelte": "^3.17.3",
    "svelte-jester": "^1.5.0",
    "svelte-preprocess": "^4.6.8",
    "svelte-preprocess-sass": "^1.0.0"
  }
}

And I am using the libraries as follow: My endpoint.js file:

import * as azureIdentity from '@azure/identity';
import * as appConfig from '@azure/app-configuration';

let baseURL;

/**
 * Get base url.
 * @returns base url.
 */
export async function getBaseURL() { 

    if (baseURL)
        return baseURL;

    // Fetch base url.
    const credential = new azureIdentity.DefaultAzureCredential();
    const client = new appConfig.AppConfigurationClient("https://agr-ecommerce-dev-appconfig.azconfig.io", credential); 
    const setting = await client.getConfigurationSetting({
       key: "eComm:Mfes:Products"
    });

    if (setting.statusCode !== 200 || setting.value == null) {
        // TODO: Write error logging logic

        return null;
    }

    return baseURL = setting.value;   
}

productAPI.js file:

import { getBaseURL } from './endpoints';
let productPath = `/products`;

/**
 * Get product information by its SKU.
 * @param {*} context `this`.
 * @param {String} sku Product SKU to return.
 * @param {Boolean} v Indicates whether variants should be returned or not.
 * @returns A product by its SKU.
 */
export async function getProductDataApi(context, sku, v = true) {
    let baseURL = await getBaseURL();

    if (baseURL == null) {
        // TODO: Write error logging logic
        throw new Error(`baseURL has null value.`);
    }

    let response = await context.fetch(`${baseURL + productPath}/${sku}?v=${v}`);
    if (response.ok) {
        let productsData =  await response.json();
        return productsData;
    } else {
        throw new Error(`Unable to load product: ${JSON.stringify(response)}`);
    }
}

I then run npm run dev and those Circular dependency warnings show.

• server
"_" is imported from external module "svelte-i18n" but never used in "src\services\i18n.js".
• client
Circular dependency: node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\api\context.js -> node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\internal\global-utils.js -> node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js       
Circular dependency: node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\api\context.js -> node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\internal\global-utils.js ->  C:\Users\JBlanco\Documents\MFE - Product\node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js?commonjs-proxy -> node_modules\@azure\identity\node_modules\@opentelemetry\api\build\src\index.js
Circular dependency: node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\api\context.js -> node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\internal\global-utils.js -> node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js   
Circular dependency: node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js -> node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\api\context.js -> node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\internal\global-utils.js ->  C:\Users\JBlanco\Documents\MFE - Product\node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js?commonjs-proxy -> node_modules\@azure\core-http\node_modules\@opentelemetry\api\build\src\index.js
> Listening on http://localhost:3000
✔ service worker (54ms)
✔ service worker (15ms)
(node:16908) Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to '0' makes TLS connections and HTTPS requests insecure by disabling certificate verification.
(Use `node --trace-warnings ...` to show where the warning was created)

I am not sure what else to include other than the project itself so you can run it lol.

[sadasant]

Thank you, @jonathanblancor ! That message was very helpful. I’ll get back with more information in some hours.

[sadasant]

@jonathanblancor I believe that for sapper to work, I would need a webpack or rollup configuration file. Would it be possible for you to share any of these configuration files with us?

It’s worth mentioning that we’ve had a similar issue in the past, and we solved it by setting the following in one of our rollup configuration files: https://github.com/Azure/azure-sdk-for-js/blob/cede8dca33de668b1b7f893b886921528a7342c6/sdk/storage/storage-blob/rollup.base.config.js#L162 I’ve copied it here for better visibility:

    onwarn(warning, warn) {
      if (
        warning.code === "CIRCULAR_DEPENDENCY" &&
        warning.importer.indexOf(path.normalize("node_modules/@opentelemetry/api")) >= 0
      ) {
        // opentelemetry contains circular references but it doesn't cause issues.
        return;
      }

      if (
        warning.code === "CIRCULAR_DEPENDENCY" ||
        warning.code === "UNRESOLVED_IMPORT"
        // Unresolved imports in the browser may break apps with frameworks such as angular.
        // Shim the modules with dummy src files for browser to avoid regressions.
      ) {
        throw new Error(warning.message);
      }
      warn(warning);
    }

Would that help? Please let us know, and thank you for your time.

[jonathanblancor]

Hi @sadasant,

Here is rollup.config.js

import path from 'path';
import resolve from '@rollup/plugin-node-resolve';
import replace from '@rollup/plugin-replace';
import commonjs from '@rollup/plugin-commonjs';
import url from '@rollup/plugin-url';
import svelte from 'rollup-plugin-svelte';
import babel from '@rollup/plugin-babel';
import { terser } from 'rollup-plugin-terser';
import config from 'sapper/config/rollup.js';
import pkg from './package.json';
import sveltePreprocess from "svelte-preprocess";
import svg from 'rollup-plugin-svg';
import { sass } from "svelte-preprocess-sass";
import json from "@rollup/plugin-json";
import cssPurge from 'ashcomm-css-purge';

const mode = process.env.NODE_ENV;
const dev = mode === 'development';
const legacy = !!process.env.SAPPER_LEGACY_BUILD;
process.env.NODE_TLS_REJECT_UNAUTHORIZED = 0; // remove later, not no prod

const onwarn = (warning, onwarn) =>
    (warning.code === 'MISSING_EXPORT' && /'preload'/.test(warning.message)) ||
    (warning.code === 'CIRCULAR_DEPENDENCY' && /[/\\]@sapper[/\\]/.test(warning.message)) ||
    (/Unused CSS selector/.test(warning.message)) ||
    (warning.code === 'PLUGIN_WARNING' && warning.pluginCode && warning.pluginCode === 'a11y-no-onchange') ||

    onwarn(warning);

const preprocess = sveltePreprocess({
    scss: {
        includePaths: ["theme"],
    },
});

export default {
    client: {
        input: config.client.input(),
        output: config.client.output(),
        plugins: [
            json(),
            svg(),
            replace({
                'preventAssignment': true,
                'process.browser': true,
                'process.env.NODE_ENV': JSON.stringify(mode)
            }),
            svelte({
                compilerOptions: {
                    dev,
                    hydratable: true
                },
                preprocess: {
                    style: sass(),
                }
            }),
            url({
                sourceDir: path.resolve(__dirname, 'src/node_modules/images'),
                publicPath: '/client/'
            }),
            resolve({
                browser: true,
                dedupe: ['svelte']
            }),
            commonjs(),
            cssPurge({
                targets: [
                    { src: '__sapper__/dev/client/FieldErrorMessage-bce587e1.css' }
                ]
            }),

            legacy && babel({
                extensions: ['.js', '.mjs', '.html', '.svelte'],
                babelHelpers: 'runtime',
                exclude: ['node_modules/@babel/**'],
                presets: [
                    ['@babel/preset-env', {
                        targets: '> 0.25%, not dead'
                    }]
                ],
                plugins: [
                    '@babel/plugin-syntax-dynamic-import',
                    ['@babel/plugin-transform-runtime', {
                        useESModules: true
                    }]
                ]
            }),

            !dev && terser({
                module: true
            })
        ],

        preserveEntrySignatures: false,
        onwarn,
    },

    server: {
        input: config.server.input(),
        output: config.server.output(),
        plugins: [
            json(),
            svg(),
            replace({
                'preventAssignment': true,
                'process.browser': false,
                'process.env.NODE_ENV': JSON.stringify(mode)
            }),
            svelte({
                compilerOptions: {
                    dev,
                    generate: 'ssr',
                    hydratable: true
                },
                preprocess: {
                    style: sass(),
                },
                emitCss: false
            }),
            url({
                sourceDir: path.resolve(__dirname, 'src/node_modules/images'),
                publicPath: '/client/',
                emitFiles: false // already emitted by client build
            }),
            resolve({
                dedupe: ['svelte']
            }),
            commonjs(),
            cssPurge({
                targets: [
                    { src: '__sapper__/build/client/FieldErrorMessage-bce587e1.css' }
                ]
            })
        ],
        external: Object.keys(pkg.dependencies).concat(require('module').builtinModules),

        preserveEntrySignatures: 'strict',
        onwarn,
    },

    serviceworker: {
        input: config.serviceworker.input(),
        output: config.serviceworker.output(),
        plugins: [
            resolve(),
            replace({
                'preventAssignment': true,
                'process.browser': true,
                'process.env.NODE_ENV': JSON.stringify(mode)
            }),
            commonjs(),
            !dev && terser()
        ],

        preserveEntrySignatures: false,
        onwarn,
    }
};

[sadasant]

@jonathanblancor hello! Thank you for sharing this.

While we discuss what to do moving forward, to confirm whether the solution we have today works, please add the onwarn code I shared above in the configuration object where Identity is being used (I’m guessing it would be inside of the service object for your case).

    // Place this code inside one of your configuration objects
    onwarn(warning, warn) {
      if (
        warning.code === "CIRCULAR_DEPENDENCY" &&
        warning.importer.indexOf(path.normalize("node_modules/@opentelemetry/api")) >= 0
      ) {
        // opentelemetry contains circular references but it doesn't cause issues.
        return;
      }

      if (
        warning.code === "CIRCULAR_DEPENDENCY" ||
        warning.code === "UNRESOLVED_IMPORT"
        // Unresolved imports in the browser may break apps with frameworks such as angular.
        // Shim the modules with dummy src files for browser to avoid regressions.
      ) {
        throw new Error(warning.message);
      }
      warn(warning);
    }

Please reply when you have a moment to see if this works for your case. I’ll come back with more information some time after your response.

Thank you so much for your time and feedback! What you’re communicating to us will help us improve.

[jonathanblancor]

That code goes on my rollup.config.js file right? Also, I have warn function already, so I'm just adding this line warning.importer.indexOf(path.normalize("node_modules/@opentelemetry/api")) >= 0

So the code looks like

const onwarn = (warning, onwarn) =>
    (warning.code === 'MISSING_EXPORT' && /'preload'/.test(warning.message)) ||
    (warning.code === 'CIRCULAR_DEPENDENCY' && warning.importer.indexOf(path.normalize("node_modules/@opentelemetry/api")) >= 0 && /[/\\]@sapper[/\\]/.test(warning.message)) ||
    (/Unused CSS selector/.test(warning.message)) ||
    (warning.code === 'PLUGIN_WARNING' && warning.pluginCode && warning.pluginCode === 'a11y-no-onchange') ||
    onwarn(warning);

[sadasant]

@jonathanblancor thank you! Does that work for you? We’re still discussing how we can improve your experience, and the experience of other users encountering the same issue.

[jonathanblancor]

Are there plans to solve that warning rather than suppressing it?

[sadasant]

@jonathanblancor we’re figuring out a path forward for this issue. I will come back as soon as I have more information. Thank you for your patience!

[sadasant]

@jonathanblancor

After finding a bit of time to investigate this and discuss it with my team, I have something to share.

When we bundle our packages using commonjs modules (such as your case), we see a circular dependency warning in the version of @opentelemetry/api that we’re using, which is 0.10.2. It seems like this won’t be the case on 0.20.0, but we will confirm in the following days. We will be following up with releases updating our packages to use @opentelemetry/api version 0.20.0 soon after we’ve finished migrating.

I have answered another issue you’ve submitted: https://github.com/open-telemetry/opentelemetry-js-api/issues/87#issuecomment-861049084

You can see our progress upgrading to @opentelemetry/api v0.20.0 here: https://github.com/Azure/azure-sdk-for-js/pull/15672

Keep in mind that, as far as we can tell, these circular dependencies shouldn’t have any functional negative impact (other than the warning log in itself). For now, we suggest ignoring this warning. Please let us know if you’re able to find a functional impact other than the warning that will help us prioritize this problem.

Thank you for your patience.

[jonathanblancor]

Thank you for the update! I'll wait.

[sadasant]

@jonathanblancor Hello, Jonathan!

I’m writing to let you know that @maorleger will take over this issue. Maor is currently working on all things related to @opentelemetry. He’s fully aware of all the information we’ve talked about so far. Please keep in mind that it may take us some days to provide a good answer. Thank you for your patience!

[jonathanblancor]

@sadasant, @maorleger any update on this?

[maorleger]

@jonathanblancor thanks for following up! Let's see where we're at:

• The circular dependency was confirmed to be on @opentelemetry side (https://github.com/open-telemetry/opentelemetry-js-api/issues/87#issuecomment-862405002)
• It looks like it was fixed 20 days ago, and @opentelemetry/api@1.0.1 contains the fix (https://github.com/open-telemetry/opentelemetry-js-api/releases/tag/v1.0.1)
• We are actively working on @azure/identity 2.0 which will automatically include this fix (as we now depend on @opentelemetry/api ^1.0.0 which would fetch 1.0.1)

Because @opentelemetry/api was in preview, @azure/identity is pinned against an exact version of @opentelemetry/api (indirectly, through core-tracing) so it will not get this fix directly.

All that to say - the issue was identified and fixed on @opentelemetry/api but it will take time for it to flow through our normal development and release process.

I reviewed @sadasant 's suggestion and I think suppressing the warning is a perfectly valid workaround to reduce bundling output noise as it has no impact at runtime. You can see how we did something similar in one of our rollup config files

I am keeping the issue open so we can revisit when @azure/identity 2.0.0 is released but I don't expect that we will patch this as the currently GA'd Identity version is pinned against a preview version of @opentelemetry/api and there are incompatibilities between those versions and 1.0.1

Hope this information helps!

[maorleger]

I reopened the circular dependency issue in @opentelemetry/api as version 1.0.1 still contained circular dependencies. Since there's nothing we can do on our side I think we can close this issue and track progress under https://github.com/open-telemetry/opentelemetry-js-api/issues/87

[ghost]

Hi @jonathanblancor. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

[ghost]

Hi @jonathanblancor, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.