Reference of require in esm build of base sdk packages #2464

legendecas · 2021-09-09T08:27:38Z

Please answer these questions before submitting a bug report.

What version of OpenTelemetry are you using?

0.25.0

Please provide the code you used to setup the OpenTelemetry SDK

index.js

import { MeterProvider, ConsoleMetricExporter } from '@opentelemetry/sdk-metrics-base';

const meterProvider = new MeterProvider({
  exporter: new ConsoleMetricExporter(),
});

// Initialize the Meter to capture measurements in various ways.
const meter = meterProvider.getMeter('test');

const counter = meter.createCounter('metric_name', {
  description: 'Example of a counter'
});

// Create a BoundInstrument associated with specified label values.
const boundCounter = counter.bind({});
boundCounter.add(10);

index.html

<!DOCTYPE html>
<html lang="en">
<head></head>
<body></body>
<script type="module" src="./index.js"></script>
</html>

What did you do?

Build the file with vite or rollup in esm mode (commonjs is not enabled) and access in browser.

> vite

What did you expect to see?

The script ran without exceptions.

What did you see instead?

Uncaught ReferenceError: require is not defined

Additional context

The text was updated successfully, but these errors were encountered:

legendecas · 2021-09-09T10:01:26Z

Typically, for packages like lodash.merge that use the pattern module.exports = ... should be able to work with typescript option esModuleInterop enabled.

With that option enabled, we can import pkg from 'pkg' from packages either using the pattern module.exports = ... or export default ....

Notably, Node.js built-in modules already adopt the usage pattern import fs from 'fs' in esm mode (https://nodejs.org/api/esm.html#esm_builtin_modules). That is to say, when we handwriting mjs files we don't have to write import * fs from 'fs' like what we are doing now (https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/test/baggage/W3CBaggagePropagator.test.ts#L25). And for npm packages, the Node.js interprets module.exports = ... as the key default in ModuleNamespace object.

import * as namespace from 'cjs'

console.log(namespace);
// [Module] { default: <module.exports> }

See https://nodejs.org/api/esm.html#esm_commonjs_namespaces for more details

So that should match the intuitions on esm interoperation with cjs.

However, with typescript, we still need the option esModuleInterop enabled to make that work. AFAICT, the downside of that options is that when the compile target is the default target 'cjs' in the OTEL project, additional helper codes are generated to make the generated code behave like what ES spec defined.

meteorlxy · 2021-09-14T02:55:37Z

As a workaround, you can enable build.commonjsOptions.transformMixedEsModules to make it work in vite now.

import { defineConfig } from 'vite';

export default defineConfig({
  build: {
    commonjsOptions: {
      transformMixedEsModules: true,
    },
  },
});

legendecas · 2021-09-14T03:02:09Z

@meteorlxy Thanks for the suggestion. However, the problem in this issue is that the esm builds officially published can not be imported with Node.js built-in ECMAScript modules support too, that is, the ESM build product is not a valid esm build product.

meteorlxy · 2021-09-14T03:07:16Z

Yes, I also noticed this problem yesterday. However, I don't think it's a good idea to enable esModuleInterop, because that option is kind of "contagious". It might require projects who depend on opentelemetry to enable esModuleInterop, too 🤔

I'd prefer to replace lodash.merge with lodash-es or some other esm-friendly alternatives.

legendecas · 2021-09-14T03:50:48Z

@meteorlxy I don't quite get a point of the "contagious" problem. As the option is just about the import behavior in enabled files, but not export behavior. Would you mind sharing any links about the "contagious" problem so that we can reach the same understanding of the options?

dyladan · 2021-09-14T17:45:28Z

the ESM build product is not a valid esm build product.

Yes, I also noticed this problem yesterday. However, I don't think it's a good idea to enable esModuleInterop, because that option is kind of "contagious". It might require projects who depend on opentelemetry to enable esModuleInterop, too 🤔

I'd prefer to replace lodash.merge with lodash-es or some other esm-friendly alternatives.

Do you have an alternative suggestion if not esmoduleinterop?

@meteorlxy I don't quite get a point of the "contagious" problem. As the option is just about the import behavior in enabled files, but not export behavior. Would you mind sharing any links about the "contagious" problem so that we can reach the same understanding of the options?

@meteorlxy provided an example in #2467. Here it is: https://github.com/meteorlxy/esmoduleinterop-issue

I would definitely prefer not to require our customers to enable es module interop if they don't want to, but providing a broken ESM build is also not acceptable. Is lodash.merge the only broken dependency? It is only used for configuration merging so it should be simple to either find an alternative or write it ourselves.

github-actions · 2021-11-15T06:38:51Z

This issue 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.

gcampbell-mythical · 2021-11-29T17:01:11Z

I'm also using vite and seeing this issue. Neither esModuleInterop nor transformMixedEsModules fixes the issue.

github-actions · 2022-01-31T06:38:02Z

This issue 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.

gcampbell-mythical · 2022-01-31T15:37:33Z

This is still an issue.

github-actions · 2022-04-04T06:39:21Z

This issue 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.

legendecas · 2022-04-04T07:53:06Z

The fix should have been released in v1.1.0.

legendecas added the bug Something isn't working label Sep 9, 2021

legendecas mentioned this issue Sep 13, 2021

fix: remove require usages in esm build #2467

Closed

github-actions bot added the stale label Nov 15, 2021

legendecas removed the stale label Nov 18, 2021

github-actions bot added the stale label Jan 31, 2022

dyladan added up-for-grabs Good for taking. Extra help will be provided by maintainers and removed stale labels Jan 31, 2022

github-actions bot added the stale label Apr 4, 2022

legendecas closed this as completed Apr 4, 2022

legendecas removed up-for-grabs Good for taking. Extra help will be provided by maintainers stale labels Apr 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reference of require in esm build of base sdk packages #2464

Reference of require in esm build of base sdk packages #2464

legendecas commented Sep 9, 2021 •

edited

Loading

legendecas commented Sep 9, 2021

meteorlxy commented Sep 14, 2021

legendecas commented Sep 14, 2021

meteorlxy commented Sep 14, 2021 •

edited

Loading

legendecas commented Sep 14, 2021 •

edited

Loading

dyladan commented Sep 14, 2021

github-actions bot commented Nov 15, 2021

gcampbell-mythical commented Nov 29, 2021

github-actions bot commented Jan 31, 2022

gcampbell-mythical commented Jan 31, 2022

github-actions bot commented Apr 4, 2022

legendecas commented Apr 4, 2022

Reference of require in esm build of base sdk packages #2464

Reference of require in esm build of base sdk packages #2464

Comments

legendecas commented Sep 9, 2021 • edited Loading

What version of OpenTelemetry are you using?

Please provide the code you used to setup the OpenTelemetry SDK

What did you do?

What did you expect to see?

What did you see instead?

Additional context

legendecas commented Sep 9, 2021

meteorlxy commented Sep 14, 2021

legendecas commented Sep 14, 2021

meteorlxy commented Sep 14, 2021 • edited Loading

legendecas commented Sep 14, 2021 • edited Loading

dyladan commented Sep 14, 2021

github-actions bot commented Nov 15, 2021

gcampbell-mythical commented Nov 29, 2021

github-actions bot commented Jan 31, 2022

gcampbell-mythical commented Jan 31, 2022

github-actions bot commented Apr 4, 2022

legendecas commented Apr 4, 2022

legendecas commented Sep 9, 2021 •

edited

Loading

meteorlxy commented Sep 14, 2021 •

edited

Loading

legendecas commented Sep 14, 2021 •

edited

Loading