opentelemetry-js
opentelemetry-js copied to clipboard
fix(api): esnext export condition is more specific than module
Which problem is this PR solving?
I'm building an application with rollup. I want to use the esnext export from @opentelemetry/api, so I add 'esnext' as an export condition. However, since the 'module' condition is always used and in the package.json it occurs before the esnext condition, the module condition will always match.
Since the esnext condition is more specific, I think it makes sense to have it occur before the module condition.
https://nodejs.org/api/packages.html#packages_conditional_exports
Short description of the changes
Move esnext export condition to the top in package.json, so that an esnext + module condition will resolve to the esnext directory.
Before
exportConditions: ['module', 'esnext'] => esm
exportConditions: ['module'] => esm
After
exportConditions: ['module', 'esnext'] => esnext
exportConditions: ['module'] => esm
Type of change
Please delete options that are not relevant.
- [x] Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
- [x] Local testing
Checklist:
- [x] Followed the style guidelines of this project
- [ ] Unit tests have been added
- [ ] Documentation has been updated
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 95.01%. Comparing base (41ba7f5) to head (d365bc8).
:warning: Report is 12 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #5458 +/- ##
=======================================
Coverage 95.01% 95.01%
=======================================
Files 303 303
Lines 7946 7946
Branches 1607 1607
=======================================
Hits 7550 7550
Misses 396 396
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@cjpearson Interestingly we were (separately) discussing dropping the build/esnext export... though that was about packages other than this @opentelemetry/api package, and in context of bumping the TypeScript transpilation target to ES2022 (https://github.com/open-telemetry/opentelemetry-js/issues/5393).
Can you show a repro, or at least your full rollup config? I'm not very familiar with the config settings involved in the various bundlers here.
Also, I'm curious what about the build/esnext output that you want/need vs. the current build/esm output. Thanks.
I wanted to use the esnext build to get the newer compilation target, since rollup also complains about the global this that is present in the esm build. So bumping the target for esm would be even better, but I thought this might work as a non-breaking solution.
I'm not actually using rollup directly, but rather through nuxt. I'll try and create a simple reproduction though.
I've created an example here: https://github.com/cjpearson/nuxt-otel-repro
Nuxt uses several plugins, so it's probably possible to narrow it down further if needed. But this app adds a single endpoint which imports trace from @opentelemetry/api and creates a span.
When building, rollup logs a bunch of warnings:
ℹ Building Nuxt Nitro server (preset: node-server, compatibility date: 2024-11-01)
node_modules/@opentelemetry/api/build/esm/metrics/NoopMeter.js (16:17): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/metrics/NoopMeter.js (16:25): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/diag.js (16:14): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/diag.js (16:22): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/diag.js (32:21): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/diag.js (32:29): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/context.js (16:14): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/context.js (16:22): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/context.js (32:21): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/api/context.js (32:29): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/baggage/internal/baggage-impl.js (16:14): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/baggage/internal/baggage-impl.js (16:22): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/baggage/internal/baggage-impl.js (32:16): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/baggage/internal/baggage-impl.js (32:24): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/diag/ComponentLogger.js (16:14): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/diag/ComponentLogger.js (16:22): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/diag/ComponentLogger.js (32:21): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/diag/ComponentLogger.js (32:29): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/context/NoopContextManager.js (16:14): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/context/NoopContextManager.js (16:22): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/context/NoopContextManager.js (32:21): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
node_modules/@opentelemetry/api/build/esm/context/NoopContextManager.js (32:29): The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.
not stale
Hi @trentm, would it be possible to take another look at this?
Assuming this is the right thing to do, we theoretically should do this for the other packages in the repo that have similar "exports":
experimental/packages/otlp-exporter-base/package.json
9: "exports": {
semantic-conventions/package.json
9: "exports": {
I think that makes sense. I've updated the PR to also include the changes for those packages
I raised this with the browser group to try to get a bundler expert opinion on this. It looks ok to me but I don't have a lot of experience with the intricacies of different bundlers and their behaviors. I don't want to accidentally do something to fix one problem that causes another.
@dyladan @cjpearson Tested with rollup 4.45.1 (latest). Thanks for doing this.
@cjpearson @dyladan is anything else needed to get this merged?
Thank you for your contribution @cjpearson! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.