feat(node): Document SEA setup#17752
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
inventarSarah
left a comment
inventarSarah
left a comment
There was a problem hiding this comment.
Thank you, the content looks good overall 👍
I left some suggestions and questions for you.
Regarding Structure:
I suggest restructuring the "ESM without import" page into two sections (add a new section for the original setup instructions).
That would help users find what they need more quickly and, maybe, make it easier for you to decide where to place which content.
|
|
||
| Keep `sea-bootstrap.cjs`, `instrument.mjs`, and `app.mjs` available on the | ||
| filesystem next to the executable. If you want a fully self-contained | ||
| executable, bundle your instrumentation and app into the SEA main instead. |
There was a problem hiding this comment.
| executable, bundle your instrumentation and app into the SEA main instead. | |
| bundle your instrumentation and app into the SEA main instead. |
It would be nice to make this more actionable -- e.g., link to docs that explain how to do this.
| }); | ||
| ``` | ||
|
|
||
| Then configure SEA to use `sea-main.cjs` as its main script and disable code |
There was a problem hiding this comment.
q: is it clear why users need to disable code cache or do we need to mention why?
There was a problem hiding this comment.
So when investigating this earlier we did need it to force node to resolve the files at runtime, but looks like we don't anymore with createRequire approach I settled on last. I will remove it!
thanks!
| </Alert> | ||
|
|
||
| When running your application in ESM mode, you will most likely want to <PlatformLink to="/install/esm">follow the ESM instructions</PlatformLink>. However, if you want to avoid using the `--import` command line option, for example if you have no way of configuring a CLI flag, you can also follow an alternative setup that involves importing the `instrument.mjs` file directly in your application. | ||
|
|
There was a problem hiding this comment.
Then create a new section for the original setup instructions. Add an H2 and name it something like "Static Import" or "Direct Import" (or whatever you want to call it)
There was a problem hiding this comment.
Settled on "Direct imports"
| As a result, the Sentry SDK will not capture data from database calls, queues, ORMs, third-party libraries, or other framework-specific data. | ||
|
|
||
| We recommend using this only if the `--import` flag is not an option for you. | ||
| We recommend using these setups only if the `--import` flag is not an option for you. |
There was a problem hiding this comment.
You can remove your changes from this alert because it now lives in the "Static import" section
| @@ -18,4 +18,4 @@ | |||
| </Alert> | |||
|
|
|||
| When running your application in ESM mode, you will most likely want to <PlatformLink to="/install/esm">follow the ESM instructions</PlatformLink>. However, if you want to avoid using the `--import` command line option, for example if you have no way of configuring a CLI flag, you can also follow an alternative setup that involves importing the `instrument.mjs` file directly in your application. | |||
There was a problem hiding this comment.
Since this page now covers 2 options, we need to update this intro.
We could write something like:
If you can't use the --import flag, choose the setup that matches your situation: static import for regular ESM apps, or the SEA bootstrap setup if you're building a Single Executable Application.
coolguyzone
left a comment
coolguyzone
left a comment
There was a problem hiding this comment.
Looks good, thanks for updating 🫡
- Update intro to describe both setup options - Move original instructions under "Direct Imports" H2 - Scope the restriction alert to direct import section only
The createRequire pattern works fine with code cache enabled, verified with a test SEA build on Node 22.
| } | ||
|
|
||
| startApp(); | ||
| ``` |
There was a problem hiding this comment.
Bug: The startApp() async function is called without a .catch() block, which can lead to an unhandled promise rejection and crash the application if an import fails.
Severity: MEDIUM
Suggested Fix
Add a .catch() block to the startApp() function call to handle potential promise rejections. This will prevent the application from crashing if a dynamic import fails.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: docs/platforms/javascript/common/install/esm-without-import.mdx#L88
Potential issue: In the `sea-bootstrap.cjs` example code, the `startApp()` async
function is invoked at the top level without any error handling. Since `startApp`
performs dynamic imports, any failure during the import process (e.g., a missing file)
will result in a rejected promise. Without a `.catch()` block to handle this rejection,
the Node.js process will terminate due to an unhandled promise rejection. This affects
users who copy this example code.
4 participants
A user asked how to get their instrumentations working in a Node.js Single Applications setup, I tried a few things and settled on this as it seems to work best and simplest way to set it up.
Not sure if needs its own dedicated page, but the user did follow the trail of not being able to use
--import, so it might be an appropriate place for it.