Skip to content

Conversation

@addaleax
Copy link
Member

Since SEA is very similar in principle to embedding functionality, it makes sense to share code paths where possible. This commit does so and addresses a TODO while doing so.

It also adds a utility to directly run CJS code to the embedder startup callback, which comes in handy for this purpose.

Finally, this commit is breaking because it aligns the behavior of require()ing internal modules; previously, embedders could use the require function that they received to do so. (If this is not considered breaking because accessing internals is not covered by the API, then this would need ABI compatibility patches for becoming fully non-breaking.)

Since SEA is very similar in principle to embedding functionality,
it makes sense to share code paths where possible. This commit does
so and addresses a `TODO` while doing so.

It also adds a utility to directly run CJS code to the embedder
startup callback, which comes in handy for this purpose.

Finally, this commit is breaking because it aligns the behavior
of `require()`ing internal modules; previously, embedders could
use the `require` function that they received to do so.
(If this is not considered breaking because accessing internals
is not covered by the API, then this would need ABI compatibility
patches for becoming fully non-breaking.)
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Feb 24, 2023
@addaleax addaleax requested a review from RaisinTen February 25, 2023 21:24
Copy link
Member

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 28, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 28, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 1, 2023
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 2, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 2, 2023
@nodejs-github-bot nodejs-github-bot merged commit 3803b02 into nodejs:main Mar 2, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 3803b02

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. single-executable Issues and PRs related to single-executable applications

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants