Circular dependency #15584
Comments
ghost
added
needs-triage
ramya-rao-a
added
Azure.Identity
Client
ghost
removed
the
needs-triage
ghost
added
the
needs-team-attention
|
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 I won’t have time to investigate today, but I’ll take a look tomorrow for sure. |
|
This is what I have: Aren't these the latest ones? |
|
@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. |
|
All I did was follow the steps in @azure/app-configuration and @azure/identity. My And I am using the libraries as follow: 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;
}
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 I am not sure what else to include other than the project itself so you can run it lol. |
|
Thank you, @jonathanblancor ! That message was very helpful. I’ll get back with more information in some hours. |
|
@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: 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. |
|
Hi @sadasant, Here is 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,
}
}; |
|
@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 // 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. |
|
That code goes on my 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); |
|
@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. |
|
Are there plans to solve that warning rather than suppressing it? |
|
@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! |
|
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: open-telemetry/opentelemetry-js-api#87 (comment) You can see our progress upgrading to @opentelemetry/api v0.20.0 here: #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. |
|
Thank you for the update! I'll wait. |
|
@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 |
|
@sadasant, @maorleger any update on this? |
|
@jonathanblancor thanks for following up! Let's see where we're at:
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! |
|
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 open-telemetry/opentelemetry-js-api#87 |
maorleger
added
issue-addressed
|
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 “ |
|
Hi @jonathanblancor, since you haven’t asked that we “ |
@azure/identityis throwing aCircular dependencywarning. I have placed the text warning and picture.I am importing
@azurelibraries 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.jsAny other information needed please let me know.
The text was updated successfully, but these errors were encountered: