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

vite build for browser pulling in node built-in modules #1892

Closed
vidarc opened this issue Jan 8, 2024 · 11 comments
Closed

vite build for browser pulling in node built-in modules #1892

vidarc opened this issue Jan 8, 2024 · 11 comments
Assignees
Labels
bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies

Comments

@vidarc
Copy link

vidarc commented Jan 8, 2024

What version of OpenTelemetry are you using?

Using latest versions of packages.

What version of Node are you using?

Node 20

What did you do?

Running vite build (using version 4 of vite) will throw the following error.

"normalize" is not exported by "__vite-browser-external", imported by "../../node_modules/@opentelemetry/instrumentation/build/esm/instrumentationNodeModuleFile.js".
file: .../node_modules/@opentelemetry/instrumentation/build/esm/instrumentationNodeModuleFile.js:16:9
14:  * limitations under the License.
15:  */
16: import { normalize } from 'path';
             ^
17: var InstrumentationNodeModuleFile = /** @class */ (function () {
18:     function InstrumentationNodeModuleFile(name, supportedVersions, patch, unpatch) {
error during build:
RollupError: "normalize" is not exported by "__vite-browser-external", imported by "../../node_modules/@opentelemetry/instrumentation/build/esm/instrumentationNodeModuleFile.js".

You will also get a warning about

[plugin:vite:resolve] Module "path" has been externalized for browser compatibility, imported by ".../node_modules/@opentelemetry/instrumentation/build/esm/instrumentationNodeModuleFile.js". See http://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.

What did you expect to see?

I would expect browser based builds to not pull in any node built-ins, so that there is no need to polyfill them and thus help to keep bundle sizes smaller.

Additional context

@vidarc vidarc added the bug Something isn't working label Jan 8, 2024
@lnmp4000
Copy link

Same issue using rollup.js

(!) Unresolved dependencies
https://rollupjs.org/troubleshooting/#warning-treating-module-as-external-dependency
path (imported by "node_modules/@opentelemetry/instrumentation/build/esm/instrumentationNodeModuleFile.js")

@pichlermarc
Copy link
Member

This is fixed by open-telemetry/opentelemetry-js#4386, the new release will take care of it (we plan to release early next week)

@pichlermarc pichlermarc added the priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies label Jan 10, 2024
@pichlermarc pichlermarc self-assigned this Jan 15, 2024
@imWildCat
Copy link

Hi @pichlermarc ! Good job!

I tried to upgrade all open telemetry packages but still seeing this error from @opentelemetry/[email protected] which contains the bad version, '@opentelemetry/instrumentation': 0.46.0(@opentelemetry/[email protected])

  /@opentelemetry/[email protected](@opentelemetry/[email protected])([email protected]):
    resolution: {integrity: sha512-JbbuOBhIfOSTOtrNU0ovgTIh3Wz05/y3RhPpLjhJBWF6M2h1u2tMjsod1/JUgrUn3mAAOLMmy1OJB23Ps0dDRg==}
    engines: {node: '>=14'}
    peerDependencies:
      '@opentelemetry/api': ^1.3.0
      zone.js: 0.11.4
    dependencies:
      '@opentelemetry/api': 1.7.0
      '@opentelemetry/instrumentation': 0.46.0(@opentelemetry/[email protected])
      '@opentelemetry/instrumentation-document-load': 0.34.1(@opentelemetry/[email protected])
      '@opentelemetry/instrumentation-fetch': 0.46.0(@opentelemetry/[email protected])
      '@opentelemetry/instrumentation-user-interaction': 0.34.1(@opentelemetry/[email protected])([email protected])
      '@opentelemetry/instrumentation-xml-http-request': 0.46.0(@opentelemetry/[email protected])
      zone.js: 0.11.4
    transitivePeerDependencies:
      - supports-color
    dev: false

@pichlermarc
Copy link
Member

Hi @imWildCat, we still need to release this repo before the bug is fixed (we released core today and it has the fix, but we have not updated the packages in this repo yet).
I'll try to get it out as soon as possible. Aiming for tomorrow.

@imWildCat
Copy link

@pichlermarc got it! thank you so much!

@starskyreverie
Copy link

+1 on this, tysm for getting to it so fast!

@pichlermarc
Copy link
Member

We finally managed to get everything sorted out. Sorry that this took so long.
It should be fixed in the latest release.

@alexandprivate
Copy link

is this solved? I still experiencing the same issue with the latest version, in my case all packages are installed by faro-react but I manually added instrumentation, api and sdk-node, am I missing something?

@pichlermarc
Copy link
Member

pichlermarc commented Jan 29, 2024

is this solved? I still experiencing the same issue with the latest version, in my case all packages are installed by faro-react but I manually added instrumentation, api and sdk-node, am I missing something?

It should be. I'm not sure how faro works, but I think it'll pull in older versions of @opentelemetry/instrumentation as the dependencies for the package will pull in ^0.34.0 of @opentelemetry/instrumentation-document-load, but the latest is 0.35.0 which does not match that range. I'm sure they'll bump the dependencies and release soon.

@alexandprivate
Copy link

Quick update, overwriting works thanks, faro-web-tracking keeps pulling 0.34.0

@eskirk
Copy link

eskirk commented Jan 29, 2024

@alexandprivate sorry for the delay on the Faro side of things - we will be bumping dependencies and cutting a release today/tomorrow.

thanks @pichlermarc for keeping us up to date and getting this fix rolled out 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies
Projects
None yet
Development

No branches or pull requests

7 participants