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

BlobServiceClient.js depends on '@opentelemetry/types'. CommonJS or AMD dependencies can cause optimization bailouts. #11550

Closed
6 tasks
blrchen opened this issue Sep 29, 2020 · 18 comments
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. Storage Storage Service (Queues, Blobs, Files)
Milestone

Comments

@blrchen
Copy link

blrchen commented Sep 29, 2020

  • Package Name: azure/storage-blob
  • Package Version: 12.1.1
  • Operating system: windows 10
  • nodejs
    • version:
  • browser
    • name/version:
  • typescript
    • version:
  • Is the bug related to documentation in

Describe the bug
I am using @azure/storage-blob in my angular app for blob upload operations. Recently I upgraded my project to angular 10 from 9, and start seeing following warnings in npm run build command output.
WARNING in C:\vso\AzureSpeed\src\AzureSpeed.WebApp\ClientApp\node_modules@azure\storage-blob\dist-esm\src\BlobServiceClient.js depends on '@opentelemetry/types'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

WARNING in C:\vso\AzureSpeed\src\AzureSpeed.WebApp\ClientApp\node_modules@azure\storage-blob\dist-esm\src\BlobServiceClient.js depends on '@azure/core-paging'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

WARNING in C:\vso\AzureSpeed\src\AzureSpeed.WebApp\ClientApp\node_modules@azure\core-http\es\src\util\utils.js depends on 'uuid/v4'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

To Reproduce
Steps to reproduce the behavior:

  1. Use latest azure/storage-blob in project created with angular 10, run npm run build

Expected behavior
No such build warnings

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Sep 29, 2020
@ramya-rao-a ramya-rao-a added Azure.Core Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Sep 29, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 29, 2020
@ghost ghost added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Sep 29, 2020
@jeremymeng
Copy link
Member

Related to https://angular.io/guide/build#configuring-commonjs-dependencies.

Looks like we could add esm output for core-paging like we did for most of our libraries.

@xirzec
Copy link
Member

xirzec commented Sep 29, 2020

I'm confused, is this related to core-paging or to OpenTelemetry?

@xirzec
Copy link
Member

xirzec commented Sep 29, 2020

The original issue looks like a duplicate of open-telemetry/opentelemetry-js#1253

@jeremymeng
Copy link
Member

I'm confused, is this related to core-paging or to OpenTelemetry?

I guess those two are causing warnings when using storage libraries.

For core-paging, something like #11563 should work?

@xirzec
Copy link
Member

xirzec commented Sep 30, 2020

Ah, I somehow missed the middle error. Yes, it seems like we can resolve the core-paging one rather easily. I'm surprised about the uuid one, since I'm pretty sure they support ESM the last time I checked.

@ramya-rao-a ramya-rao-a added feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Sep 30, 2020
@ramya-rao-a ramya-rao-a added this to the Backlog milestone Sep 30, 2020
@ramya-rao-a ramya-rao-a removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Sep 30, 2020
@ramya-rao-a
Copy link
Contributor

Should we log an upstream issue for uuid like the one logged for open telemetry?

@jeremymeng
Copy link
Member

Ah, I somehow missed the middle error

lol, I somehow managed to miss the uuid warning. yes it supports esm. I wonder whether it's our usage pattern though. we are not importing the default, but v4 from uuid.

import { v4 as uuidv4 } from "uuid";

@xirzec
Copy link
Member

xirzec commented Sep 30, 2020

Should we log an upstream issue for uuid like the one logged for open telemetry?

They already support it: https://github.com/uuidjs/uuid/blob/ac1b3afbf410e2dab259a1b7df0f27da919aabff/package.json#L32

They even support the fancy version: https://github.com/uuidjs/uuid/blob/ac1b3afbf410e2dab259a1b7df0f27da919aabff/package.json#L24

I think we might need to update to latest since the version we're on has a slightly different setup:

https://unpkg.com/[email protected]/package.json

Notably, it's missing the module version in the fancy form, which might be the problem?

@jeremymeng - I believe importing v4 is fine since we're not doing a deep import, it's no different than importing * as uuid and then writing const uuidv4 = uuid.v4;

@jeremymeng
Copy link
Member

Right I do not see the uuid warning in my testing today, likely 8.3.0 is pulled down.

@jeremymeng jeremymeng modified the milestones: Backlog, MQ-2020 Oct 13, 2020
@ljian3377 ljian3377 removed the Client This issue points to a problem in the data-plane of the library. label Oct 21, 2020
@ramya-rao-a ramya-rao-a added the Client This issue points to a problem in the data-plane of the library. label Dec 7, 2020
@bgcc93
Copy link

bgcc93 commented Dec 15, 2020

I am experiencing the same issue with the package @azure\storage-file-datalake on version 12.2.0.

Warning: [...]\node_modules@azure\storage-file-datalake\dist-esm\storage-file-datalake\src\DataLakeServiceClient.js depends on '@opentelemetry/api'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

Could it be some peer dependency I need to install? I am only making use of @azure\storage-file-datalake right now, not @azure/storage-blob

@ramya-rao-a ramya-rao-a removed this from the MQ-2020 milestone Dec 23, 2020
@ramya-rao-a ramya-rao-a added this to the [2021] February milestone Dec 23, 2020
@jeremymeng
Copy link
Member

@bgcc93 @azure\storage-file-datalake and many of our SDK client libraries depend on @azure\core-tracing which in turn depends on @opentelemetry/api. You are not missing any peer dependency.

@jeremymeng
Copy link
Member

Looks that the linked opentelemetry issue has been fixed. We should check back after upgrading to the version that contains the fix.

@ramya-rao-a ramya-rao-a assigned maorleger and unassigned xirzec and jeremymeng Jun 4, 2021
@ramya-rao-a ramya-rao-a modified the milestones: Backlog, [2021] July Jun 4, 2021
@ramya-rao-a
Copy link
Contributor

Upstream issues have been fixed and updates have been released

@maorleger Lets make sure we get on the latest 0.20.0 of @opentelemetry/api across all our packages for the July release and confirm that this issue is resolved

@maorleger
Copy link
Member

To repro this I created a new angular app and added @azure/core-tracing (latest version is preview.11 on npm), added some usage of core tracing and ran npm run build. Summarized output below:

otel-angular-test master % npm i @azure/core-tracing
+ @azure/[email protected]
added 1 package, removed 29 packages, updated 5 packages and audited 1323 packages in 5.51s
otel-angular-test master % npm run build

> [email protected] build /home/mleger/workspace/otel-angular-test
> ng build

✔ Browser application bundle generation complete.
✔ Copying assets complete.
✔ Index html generation complete.

Initial Chunk Files               | Names         |      Size
main.0d526b4ed5d927c5864b.js      | main          | 134.38 kB
polyfills.5bfcb9c68535336c5861.js | polyfills     |  35.98 kB
runtime.f1870f2033cec01c3be2.js   | runtime       |   1.02 kB
styles.31d6cfe0d16ae931b73c.css   | styles        |   0 bytes

                                  | Initial Total | 171.38 kB

Build at: 2021-06-25T16:57:16.010Z - Hash: 283ddf7a8e31ee73abe1 - Time: 9917ms

Warning: /home/mleger/workspace/otel-angular-test/node_modules/@azure/core-tracing/dist-esm/src/interfaces.js depends on '@opentelemetry/api'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

Then I switched to our nightly version which contains the latest @opentelemetry/api dependencies:

otel-angular-test master % npm i @azure/core-tracing@dev
+ @azure/[email protected]
removed 2 packages, updated 2 packages and audited 1322 packages in 5.246s
otel-angular-test master % npm run build 
> [email protected] build /home/mleger/workspace/otel-angular-test
> ng build

✔ Browser application bundle generation complete.
✔ Copying assets complete.
✔ Index html generation complete.

Initial Chunk Files               | Names         |      Size
main.458876f477f5e827becc.js      | main          | 134.16 kB
polyfills.5bfcb9c68535336c5861.js | polyfills     |  35.98 kB
runtime.f1870f2033cec01c3be2.js   | runtime       |   1.02 kB
styles.31d6cfe0d16ae931b73c.css   | styles        |   0 bytes

                                  | Initial Total | 171.15 kB

Build at: 2021-06-25T16:57:44.822Z - Hash: c40ebc49b590b9aa0885 - Time: 9662ms
otel-angular-test master % 

Finally (and I will spare you the output) I packed and installed @azure/storage-blob from source (as well as @azure/core-tracing from source) and can confirm no warnings. I believe this issue will be resolved within the next release cycle.

@maorleger
Copy link
Member

@blrchen a nightly release of @azure/storage-blob has been released and I was wondering if you could help us out by trying it and seeing if the issue is resolved?

If you can give it a shot, the version you want to install is:

npm install @azure/[email protected]

Feel free to let me know if the issue still persists - I tried it now and did not see the warning.

Thanks!

@adam-dziomdziora-cybercom

@maorleger it actually did resolve the issue. (installing version 12.7.0-alpha.20210707.1)

@blrchen
Copy link
Author

blrchen commented Jul 12, 2021

@maorleger thanks for letting me know, new version is working for me.

@maorleger
Copy link
Member

Thanks folks! I will close this issue now that it's confirmed, and this fix will be released when @azure/storage-blob 12.7.0 is released.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

8 participants