[langhost/node] More ESM support: nodeargs, main entrypoints, and tests
#8655
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lukehoban
force-pushed
the
lukehoban/moreesm
branch
from
b73a832
to
ba09f6d
Compare
lewisl9029
reviewed
Dec 29, 2021
lukehoban
force-pushed
the
lukehoban/moreesm
branch
from
2f6ddd4
to
421fef5
Compare
lukehoban
changed the title
nodeargs, main entrypoints, and tests
This was referenced Dec 30, 2021
|
Unfortunately running out of time to properly review this till Monday. |
t0yv0
reviewed
Jan 4, 2022
|
It's looking good but I'm having trouble running tests locally and verifying a few things due to #8678 - looking into that first. |
t0yv0
approved these changes
Jan 4, 2022
This follows up on the changes in #7764 with three additions: * Support for passing args directly to node so that ESM loaders and other node process level configuration can be accessed via Pulumi * Support the same default behaviour for loading a ESM module from Pulumi as for node <program>, including support for resolving main entrypoint from package folder. * Add test cases for .js, pre-complied .ts and ts-node-based `.ts. This includes test cases that show how to use top-level await in Node.js (which is only possible inside ESM), addressing #5161. Using ESM via the default ts-node-based TypeScript support is a little tricky, as it is dependent on experimental loader hook support in Node, upon which partially in-progress ts-node support has been added. We cannot make these the default experience yet, but the examples here show how users can configure things themselves to access these features. Once this support solidifies and we can rely on it in all supported Node and TypeScript versions, we may be able to update templates to support more of this by default.
lukehoban
force-pushed
the
lukehoban/moreesm
branch
from
3f2a791
to
97da26a
Compare
lukehoban
added a commit
that referenced
this pull request
Jan 17, 2022
In #7764 and #8655 we added support for ESM entrypoints. However, ESM "default exports" were handled just as "normal" in Node.js dynamic import of ESM - as a `default` proeprty in the export object. This is not a particularly useful behaviour for Pulumi program entry points, and doesn't quite match some of the special logic we apply to non-object exports in CommonJS modules (invoking exported functions, and then awaiting exports promises). Instead, this change adds support for default exports, treating the default export (if present) as the full returned export value. It is for now an error to have both a default export and named exports, since it is unclear what this should mean. In the future, we could potentially relax this and define how these two sets of exports are merged. This is technically a breaking change from the support added in the recent releases, but only in a narrow case, and in that case the Pulumi stack exports were almost certainly not what the user wanted. Fixes #8725, which includes a motivating example where this is ~necessary.
This was referenced Jan 17, 2022
|
@lukehoban Does this change (specifically the nodeargs part, allowing the passing of various --experimental-* flags to node) enable full support for tsconfig's baseUrl/paths? It seems like I'm getting closer to getting this working, but I'm not all the way there. |
|
Would it be possible to file this as an issue? That'd be very helpful for us. |
lukehoban
added a commit
that referenced
this pull request
Jan 23, 2022
In #7764 and #8655 we added support for ESM entrypoints. However, ESM "default exports" were handled just as "normal" in Node.js dynamic import of ESM - as a `default` proeprty in the export object. This is not a particularly useful behaviour for Pulumi program entry points, and doesn't quite match some of the special logic we apply to non-object exports in CommonJS modules (invoking exported functions, and then awaiting exports promises). Instead, this change adds support for default exports, treating the default export (if present) as the full returned export value. It is for now an error to have both a default export and named exports, since it is unclear what this should mean. In the future, we could potentially relax this and define how these two sets of exports are merged. This is technically a breaking change from the support added in the recent releases, but only in a narrow case, and in that case the Pulumi stack exports were almost certainly not what the user wanted. Fixes #8725, which includes a motivating example where this is ~necessary.
lukehoban
added a commit
that referenced
this pull request
Jan 23, 2022
In #7764 and #8655 we added support for ESM entrypoints. However, ESM "default exports" were handled just as "normal" in Node.js dynamic import of ESM - as a `default` proeprty in the export object. This is not a particularly useful behaviour for Pulumi program entry points, and doesn't quite match some of the special logic we apply to non-object exports in CommonJS modules (invoking exported functions, and then awaiting exports promises). Instead, this change adds support for default exports, treating the default export (if present) as the full returned export value. It is for now an error to have both a default export and named exports, since it is unclear what this should mean. In the future, we could potentially relax this and define how these two sets of exports are merged. This is technically a breaking change from the support added in the recent releases, but only in a narrow case, and in that case the Pulumi stack exports were almost certainly not what the user wanted. Fixes #8725, which includes a motivating example where this is ~necessary.
justinvp
added a commit
to pulumi/pulumi-hugo
that referenced
this pull request
Sep 9, 2022
pulumi/pulumi#8655 added support for a `nodeargs` runtime option for Node.js projects, but we never explicitly documented it. This PR does that.
justinvp
added a commit
to pulumi/pulumi-hugo
that referenced
this pull request
Sep 9, 2022
pulumi/pulumi#8655 added support for a `nodeargs` runtime option for Node.js projects, but we never explicitly documented it. This PR does that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This follows up on the changes in #7764 with three additions:
nodeso that ESM loaders and other node process level configuration can be accessed via Puluminode <program>, including support for resolving main entrypoint from package folder..js, pre-complied.tsandts-node-based `.ts.This includes test cases that show how to use top-level await in Node.js (which is only possible inside ESM), addressing #5161.
Using ESM via the default
ts-node-based TypeScript support is a little tricky, as it is dependent on experimental loader hook support in Node, upon which partially in-progress ts-node support has been added. We cannot make these the default experience yet, but the examples here show how users can configure things themselves to access these features. Once this support solidifies and we can rely on it in all supported Node and TypeScript versions, we may be able to update templates to support more of this by default.