-
Notifications
You must be signed in to change notification settings - Fork 207
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
feat(agoric-cli): Support Node.js ESM deploy scripts #3686
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, modulo one little rename.
packages/agoric-cli/lib/deploy.js
Outdated
: // eslint-disable-next-line import/no-dynamic-require | ||
require(modulePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do the renaming needed to make this suggestion possible:
: // eslint-disable-next-line import/no-dynamic-require | |
require(modulePath); | |
: esmRequire(modulePath); |
packages/agoric-cli/lib/deploy.js
Outdated
|
||
// Use Node.js ESM support if package.json of template says "type": | ||
// "module". | ||
const packageFile = pathResolve('package.json'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, another thing: the package.json
should be searched starting from the moduleFile
, since it is possible that the moduleFile
is in a different subpackage than the top-level package.json
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resolved this by bringing in a utility in compartment mapper that searches for the package.json above any file.
refs: #527
This adds support for Node.js ESM deploy scripts, allowing dapps to be created
or refactored using Node.js ESM instead of the
standardthings/esm
emulation.To test this feature, I created a
nesm
branch of thedapp-fungible-faucet
dapp that was converted to Node.js ESM with
"type": "module"
inpackage.json
and verified through a console log thatagoric deploy
chose touse the Node.js ESM implementation. For good measure, I added a temporary
useless reference to
import.meta.url
, which is not properly emulated by-r esm
.This necessitated an additional feature to
agoric init
, to specify thebranch to use from the template dapp.