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

fix(node): Update ESM instruction with --import @sentry/node/import #10028

Closed
wants to merge 1 commit into from

Conversation

andreiborza
Copy link
Member

@andreiborza andreiborza commented May 14, 2024

This has been a regression from the previous version of the docs where we mentioned calling node with --loader @sentry/node/import.

Node itself marks --loader as experimental and suggests to use --import instead.

Ref: getsentry/sentry-javascript#12033

Copy link

vercel bot commented May 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2024 8:54pm

Copy link

codecov bot commented May 14, 2024

Bundle Report

Changes will decrease total bundle size by 12 bytes ⬇️

Bundle name Size Change
sentry-docs-edge-server 458.69kB 3 bytes ⬇️
sentry-docs-server 7.38MB 3 bytes ⬇️
sentry-docs-client 6.16MB 6 bytes ⬇️

Copy link
Contributor

@lizokm lizokm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update! I have a few language and structure change suggestions.

When running your application in ESM mode, you can't use `require()` to load modules. Instead, you have to use the `--import` command line options to load a module before the application starts:

Adjust the Node.js call for your application to use the [--import](https://nodejs.org/api/cli.html#--importmodule) parameter and point it at `instrument.js`, which contains your `Sentry.init()` code:
When running your application in ESM mode, you need to pass an [--import](https://nodejs.org/api/cli.html#--importmodule) command line option to register the SDK's ESM module loader so that it can properly instrument libraries.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When running your application in ESM mode, you need to pass an [--import](https://nodejs.org/api/cli.html#--importmodule) command line option to register the SDK's ESM module loader so that it can properly instrument libraries.
Most node applications today are either written in CommonJS (CJS), or compiled to CJS before being ran. While CommonJS uses `require()` to load modules, we recommend using the `instrument.js` file at the top of your application installation method instead.
However, if your application runs in ESM mode, this method won't work unless you run your application with `--import @sentry/node/import`. Note, that even if your application is written in ESM (using `import`), it may still be _run_ in CJS. (Almost all applications written in TypeScript are compiled to CJS before being ran, for example.) If this is the case, follow the [CommonJS instructions](../commonjs).
## How to Run Your Application
To run your application in ESM mode, you'll need to pass an [--import](https://nodejs.org/api/cli.html#--importmodule) command line option like the one below to register the SDK's ESM module loader so that it can properly instrument libraries.

Comment on lines +19 to +21
<Alert level="warning">
We do not support ESM in Node versions before 18.19.0.
</Alert>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Alert level="warning">
We do not support ESM in Node versions before 18.19.0.
</Alert>
<Alert level="warning">
We don't support ESM in any versions of Node before 18.19.0.
</Alert>

We do not support ESM in Node versions before 18.19.0.
<Alert level="warning">
We do not support ESM in Node versions before 18.19.0.
</Alert>

## When to use this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## When to use this

I think this whole section makes more sense as an introduction at the top of the page.

We do not support ESM in Node versions before 18.19.0.
<Alert level="warning">
We do not support ESM in Node versions before 18.19.0.
</Alert>

## When to use this

Most node applications today are either written in CommonJS (CJS), or compiled to CJS before running them.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Most node applications today are either written in CommonJS (CJS), or compiled to CJS before running them.


## When to use this

Most node applications today are either written in CommonJS (CJS), or compiled to CJS before running them.
CommonJS uses `require()` to load modules. Our recommended installation method when using CommonJS is to require the `instrument.js` file at the top of your application. However, if your application is run in ESM mode, this will not work. In this case, you can follow the [ESM docs](./esm).
CommonJS uses `require()` to load modules. Our recommended installation method when using CommonJS is to require the `instrument.js` file at the top of your application. However, if your application is run in ESM mode, this will not work without running your application with `--import @sentry/node/import`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CommonJS uses `require()` to load modules. Our recommended installation method when using CommonJS is to require the `instrument.js` file at the top of your application. However, if your application is run in ESM mode, this will not work without running your application with `--import @sentry/node/import`.


Note that even if your application is written in ESM (using `import`), it may still be _run_ in CJS. For example, almost all applications written in TypeScript are compiled to CJS before running them. In this case, you should follow the CommonJS instructions.
Note that even if your application is written in ESM (using `import`), it may still be _run_ in CJS. For example, almost all applications written in TypeScript are compiled to CJS before running them. In this case, you should follow the [CommonJS instructions](../commonjs).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Note that even if your application is written in ESM (using `import`), it may still be _run_ in CJS. For example, almost all applications written in TypeScript are compiled to CJS before running them. In this case, you should follow the [CommonJS instructions](../commonjs).

@andreiborza
Copy link
Member Author

We decided not to proceed with this approach, thanks for the review @lizokm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants