ES Module support for Oclif v2 about core HOT 21 CLOSED

Wow @typhonrt - first, let me thank you for taking the time to contribute and for being so thorough. Now that you wrapped up the explanation, let me finish digesting this. I'm about halfway and still need to test some stuff out. I just wanted to engage to let you know we are looking at this. I agree that ESM is valuable and if we are going to add it, now is the time.

from core.

Merged. I'm going to close this out. File any additional issues for anything else you find.

from core.

In this final post I will discuss a few quality of life aspects for ESM and Oclif in general.

• support file URL for runtime entry point
• add flush to main export of @oclif/core

These minor quality of life changes regard programmatic control of invoking Oclif from an ESM context. This is the bootstrap code from test-cli-modern

#!/usr/bin/env node
import url   from 'url';
import flush from '@oclif/core/flush.js';
import { run, Errors } from '@oclif/core';

run(void 0, url.fileURLToPath(import.meta.url))
.then(flush)
.catch(Errors.handle);

which can be condensed to:

#!/usr/bin/env node
import { flush, run, Errors } from '@oclif/core';

run(void 0, import.meta.url)
.then(flush)
.catch(Errors.handle);

In particular it is reasonable to support file URLs as string & URL so that import.meta.url can be passed directly to the main run entry point. As things go when executing from an ESM context module / module.parent are not available so one must pass in a file URL via import.meta.url to load the appropriate config. This would involve checking that options is a string formatted as a file URL or URL and using url.fileURLToPath in src/main.ts.

The other suggestion of adding flush to the main export just streamlines import of all resources commonly necessary to invoke Oclif programmatically.

Well that wraps everything up for now.. I remain hopeful to interface with the Oclif team before the v2 launch. IMHO launching without ESM support will bring a significant pain point to Oclif midstream during the v2 lifecycle; probably sooner than later as the move to ESM will be rapid for certain parts of the Node ecosystem.

from core.

Deploying an ESM CLI (06/01/21)

Please review the following complete test cli repos which feature how to set up ESM Oclif v2 CLIs. Code coverage is gathered and the tests show how to use chai-as-promised and fancy-test to test the main package export programmatically and cross-spawn to test the bin entry point. Currently @oclif/test has not been updated for Oclif v2 yet.

• test-cli-modern / everything works as expected (Node 12.20+ and Node 14.13+)
• test-cli-cjs-interop / workaround for CJS named exports (widest Node support; Node 12.17+ and 14.0+)

Deployment of an ESM CLI on NPM for Oclif v2 does have a problematic area to overcome presently.

While there is newly added ESM support to @oclif/core v0.5.10+ the rest of the Oclif v2 infrastructure and plugins are
not updated to use @oclif/core yet. This is somewhat problematic in using @oclif/dev-cli v1.26.0 and in particular the
oclif-dev manifest CLI command in prepack or prepublishOnly NPM scripts. There is a workaround though to
publish ESM Oclif v2 CLIs using the oclif-dev manifest command. It requires installing @oclif/dev-cli v1.26.0 as a devDependency then manually updating @oclif/config which is the v1 version depended on by @oclif/dev-cli. ESM support has been back-ported with a hard fork of @oclif/config v1 in this repository.

The contents of the lib directory from the above repository needs to be copied into node_modules/@oclif/config/lib of your CLI project / repo as it adds ESM config loading support to @oclif/config v1. The oclif-dev manifest command in the prepack or prepublishOnly NPM script will then complete successfully when publishing a CLI.

Note: This workaround only works with oclif-dev manifest command and not the README command. Please review the above example CLI repos as they are setup for publishing an ESM Oclif v2 CLI to NPM with the additional step described above to update @oclif/config v1 manually for the time being.

When Oclif v2 fully launches no manual workarounds will be necessary.

For a non-trivial example of an ESM Oclif v2 CLI that is published on NPM you can review my ongoing project fvttdev. fvttdev uses Oclif middleware that I am also developing which adds quite a few capabilities for ESM CLI development. Both are presently in stable, but alpha stages of development, so nice to have things like documentation and such is forthcoming.

from core.

Hi @phibar. Alas, I have been working on other projects other than my CLI based project, so haven't been keeping up with the current state of oclif v2. It's also a tough spot as I spent ~180 hours on researching & implementing the initial ESM support for core. Given the organization behind oclif it's hard for me to justify spending too much more time pro bono so to speak as an outside contributor. Certainly back when I completed this initial work the other plugins for v2 were not complete and there were noted incompatible ones hence the extra instructions above with workarounds.

Do update the repo link above and I can take a quick look to see if I see anything.

from core.

So I have finished the comprehensive test suites demonstrating ESM Oclif CLIs based on my fork of @oclif/core and three variations which provide the widest support to what can be considered modern ESM on Node. The tests are run via Github Actions across a matrix supporting macos-latest, ubuntu-latest, windows-latest on various Node versions from 12.0.0 to 15.x. I fully believe the thoroughness of the test suites prove that ESM support is viable for Oclif v2. The test suites invoke the test CLI programmatically and through use of spawn invoking local bin bootstrap, the CLI installed as a dev dependency / NPM script, and in the Github Action CI / CD with the CLI installed globally.

The test suites have detailed information about what limitations they address, so please refer to the README of each:

• test-cli-modern / everything works as expected (Node 12.20+ and Node 14.13+)
• test-cli-cjs-interop / workaround for CJS named exports (widest Node support; Node 12.17+ and 14.0+)
• test-cli-experimental-modules / usage of --experimental-modules + workaround for CJS named exports (not recommended / demo only for Node 12.0.0+ support)

The "modern" test suite works on Node 12.20.0+ and 14.13.0+ as those are the versions that work as expected for modern ESM on Node. It is recommended that in regard to the Oclif CLI / project creator that the modern version be the default project style created for any ESM based CLI and set the engines attribute appropriately in package.json.

Proper documentation and example projects can be offered allowing end users to manually configure an ESM CLI for wider ecosystem support addressing two caveats.

The first caveat is that until Node 12.20.0 and 14.13.0 CommonJS named exports can not be imported as expected via standard ESM mechanisms. Instead of import { Command } from '@oclif/core' one must do the following import oclif from '@oclif/core' then subsequently reference Command as oclif.Command. This small workaround allows ESM CLIs to be launched on Node 12.17.0+ and all of 14.0.0+ without the need for --experimental-modules.

The second caveat is that below Node 12.17.0 --experimental-modules must be used as an argument to Node to support ESM. This does not preclude launching an Oclif ESM CLI, but adds a little special setup which is covered in test-cli-experimental-modules.

Regarding code coverage of the Mocha ESM tests. nyc presently does not support code coverage for tests written in ESM, but a drop in replacement of c8 handles code coverage and the test repos use Codecov to provide a coverage report w/ badges available at the top of the README. This doesn't affect @oclif/core which will continue to use nyc, but end user CLIs that utilize all ESM including tests will need to use c8.

I have shown with these test suites that ESM CLIs are viable from Node version 12.0.0+ though the recommended "modern" ESM experience should be the default and works on the latest Node 12 & 14 LTS versions. It is important to verify that there is full support for the LTS versions, but it should be noted that adding ESM support to Oclif v2 is about the now and the future of Node & Oclif.

In my next post I'll go through a detailed breakdown of my ESM @oclif/core fork regarding what needs to be changed. ~185 lines of additions / removals / changes brings ESM support to Oclif. The main bulk of the modifications are in @oclif/core, but of course there will be a few small changes necessary in some of the ancillary Oclif plugins.

from core.

Now I would like to discuss the essential changes to @oclif/core that enables ESM support. This post is long and thorough and it seems reasonable to make this an open discussion with a central location before making any pull requests. I am more than open to address all concerns before making any pull requests. This post will break down the source and testing changes along with one non-essential addition that I see as a useful feature addition that was easy to implement. In a further post I'll discuss the aspects that will need minimal modifications in other Oclif plugins / repos along with a few potential quality of life changes in regard to ESM usage of Oclif.

My fork is located here with a separate "fork of a fork" compiled version with the lib directory checked in so that it can be linked from Github without publishing on NPM. This also has the benefit that it can act as a drop in replacement of @oclif/core.

I will also briefly reference my ESM Oclif middleware and active ESM CLI in development called fvttdev for an example use case.

A file comparison between the fork and @oclif/core main repo can be viewed here.

The source files that require changes are:

• src/config/config.ts
• src/config/plugin.ts
• src/help/util.ts
• src/interfaces/command.ts
• src/interfaces/config.ts
• src/interfaces/plugin.ts
• src/main.ts

New source file:

• src/module-loader.ts

One module dependency and two developer dependencies in:

• package.json

I'll start with the interface files: command.ts, config.ts, and plugin.ts

Both config.ts and plugin.ts add a boolean isESM. When the associated package.json is loaded this will be set to true if "type": "module" is set otherwise it is false. The other changes to the interface files including command.ts and plugin.ts set load() and findCommand() to return a Promise. These method signature changes to asynchronous regard being able to load ESM by dynamic import or CJS / TS by require w/ dynamic import necessitating async handling.

Edit (06/01/21): The following links where applicable have been updated to point to the now merged / actual source code in @oclif/core itself which provides ESM support. Given ongoing work on Oclif v2 these links may not accurately resolve, but should get to the approximate area where changes have occurred.

It makes sense to discuss the new file src/module-loader.ts next as it is utilized in src/config/config.ts, src/config/plugin.ts, and src/help/util.js. It made the most sense to create a way to load a file / module path and combine all of the loading logic in one class rather than repeat similar patterns of combining dynamic import / require loading ad hoc across these files. This provides one place to make modifications to how loading occurs. One of the encapsulation aspects is using the built in Node url module and pathToFileURL for dynamic import which is necessary to correctly load absolute file paths on Windows. It also cleanly encapsulates the workaround for getting dynamic import to work when a Typescript config targets CommonJS.

This workaround is necessary as dynamic import / import("...") is transpiled by tsc into something like Promise.resolve().then(() => require("...")); which does not work for importing ESM. The workaround I chose to implement does not require a change to the build process. In module-loader.ts dynamic import is preserved by creating a new Function (essentially eval) by const _importDynamic = new Function('modulePath', 'return import(modulePath)') and is only accessed internally in ModuleLoader.importDynamic and used by ModuleLoader.load and ModuleLoader.loadWithData. Now there is awareness recently on the Typescript issues forum regarding possibly adding a flag in tsconfig.json to not transpile dynamic import when the target is CommonJS. In a future release of Typescript this may be available and this small workaround can be removed. Please see: microsoft/TypeScript#43329

The two methods load and loadWithData are used with an Oclif config or plugin config plus a module path. The reason for loadWithData is primarily for debug logging purposes such as in runHook in config.ts which logs whether import() or require was used to load the hook plus the file path.

Now I would like to comment on the only non-essential feature added which is found in ModuleLoader. It wasn't hard to implement and adds functionality to loading hooks and the custom help class. That is the ability to specify in the Oclif config in package.json for a hook or custom help class to be an actual module or module export installed rather than just a local source file. This allows 3rd party modules to be loaded directly without having to re-export any resources from a local file. A use case for this is explicitly defining hook execution order for init / initializing. In the case of my Oclif middleware the init hook from the middleware needs to load first before the local CLI init hook, so order matters. Also I provide a default custom help class which can be directly referenced in the config. You can see this in use in fvttdev in its package.json.

To accomplish this and maintain present functionality of loading local files ModuleLoader.resolvePath first tries require.resolve on the passed in path / module. If it fails and throws in the catch statement the local loading code is used instead to determine a local file path. It should be noted that to support ESM loading from a 3rd party module it is necessary to determine its type; either a *.mjs file or having "type": "module" set in the associated package.json. This is where the one module addition get-package-type has been added to @oclif/core. get-package-type is from a maintainer of nyc and used there and generally widely across the Node ecosystem to accomplish this task. This is found in ModuleLoader.isPathModule. You can read about the initial announcement of get-package-type at the bottom of this issue:
nodejs/node#49446

With an understanding of the ModuleLoader addition I'll comment on changes to src/config/config.ts and src/config/plugin.ts

The largest flow control change is to runHook in config.ts which needs to be restructured to support dynamic import and require usage via ModuleLoader. The previous Promise.all and Array.map code was replaced with synchronous looping with two awaits for ModuleLoader.loadGetData and the search internal function. This ensures that loading with dynamic import and require executes synchronously. The old Promise.all / Array.map control flow had execution of hooks out of order when adding dynamic import due to the nested anonymous async function in .map() with the await from dynamic import. I do believe the new runHook code is simpler and captures the intention of loading hooks in order. Each hook starts / finishes before the next one is called.

The old code only worked due to the synchronous nature of require() and underlying implementation in Node. If you run Oclif with env DEBUG=* to enable debug logging you'll actually see log statements for all the hooks (init, etc.) starting then each resolving in order. However with the added await / async addition for dynamic import this order is not maintained. The nice thing about the new runHook method is that each hook starts / finishes before advancing to the next in order and this can be seen in log statements.

The changes to plugin.ts involve the findCommand where loading of commands occurs with either ModuleLoader.importDynamic or require depending on whether "type": "module" is set in the Oclif plugins package.json.

The following is a discussion on the changes to loading custom help classes and how async loading affects the help subsystem.

To load custom help classes that may be ESM getHelpClass loadHelpClass in src/help/util.ts needs to be changed to async for potential dynamic import use by ModuleLoader.load.

So accordingly in src/main.ts the usage of loadHelpClass needs to be async. Now you'll notice that there is an await on line 50 await help.showHelp(argv). This has no affect on the current default implementation of the Help / HelpBase implementation in @oclif/core - src/help/index.ts which is synchronous.

I propose that the help subsystem be made asynchronous as custom help implementations may need to load the command class via the command config load method which is now async. To test this functionality and limit the changes in the @oclif/core fork I rewrote Help / HelpBase in my middleware adding the async implementation. You can see in the my middleware DynamicCommandHelp implementation the usage of config.load. This is the main requirement why the help subsystem should be made asynchronous. All other execution aspects of Oclif are async, so making the help subsystem async is reasonable.

Besides the use case mentioned above I have a practical real world use case that requires async aspects to function. In my Oclif middleware I have a custom Command base class DynamicCommand which allows flags to be added dynamically to commands by other Oclif plugins installed / initialized. In my real world CLI fvttdev which is a developer CLI for Foundry VTT I'm basically creating something like Snowpack for a specific platform. I provide a bundle command which uses Rollup under the hood to build a developers project. I wrap several different Rollup plugins as Oclif plugins and each of these plugins may add flags dynamically to the bundle command. Since the CLI is multi-command these Rollup Oclif plugins are only loaded when the bundle command is executed or when the help action is activated for the bundle command, so this dynamic loading of plugins which adds flags to the bundle command utilizes the proposed async functionality of the help subsystem.

The test files that require changes are:

• test/help/util.test.ts
• test/helpers/init.js

These changes for loadHelpClass tests which is now async simply requires handling the async aspects correctly. To accomplish this chai-as-promised is added along with @types/chai-as-promised to devDependencies. In test/helpers/init.js chai-as-promised is initialized.

The above discusses the essential changes to @oclif/core to support ESM. Further work to be done revolves around a discussion about the help subsystem going asynchronous. 185 lines of additions / changes / removal brings ESM support to Oclif v2 while maintaining all existing functionality.

In the next post I'll discuss other small modifications which will need to occur in other Oclif plugins.

from core.

In this post I will discuss some of the ramifications I'm aware of for the ESM changes in regard to the other mainline Oclif plugins. The main concern is the asynchronous loading aspects and the async help subsystem including the getHelpClass utility function. Now I'm not aware of any public v2 repo locations or branches for the other plugins like plugin-help and dev-cli. Those two in particular I'm sure will be updated to work with @oclif/core for the v2 launch. I'll be making these comments based on my initial prototyping of ESM in Oclif v1 and what I saw that needed to be updated.

plugin-help will need to be updated for the async help subsystem / getHelpClass usage which will likely be a couple of lines like the modifications to src/main.ts in @oclif/core.

dev-cli - Will need the readme command updated to work with the async help subsystem.

oclif- The main oclif CLI will need to be updated regarding project creator aspects adding ESM as an option.

test - May need some changes to support ESM testing and async aspects. Probably this will involve chai-as-promised. It's hard to tell as the public repo / version still is using Oclif v1.

Most of these changes are relatively minor. The readme command support in dev-cli probably being the more involved change.

As mentioned in the first post I'd be glad to help assist in updating any of the main plugins that require changes due to the ESM loading addition to @oclif/core.

Perhaps point to any of the v2 repo / branches for the plugins when they are available for beta consumption and I'll do a full analysis.

from core.

Now I'm not aware of any public v2 repo locations or branches for the other plugins like plugin-help and dev-cli

We will be doing a major version bump of all oclif repositories when they transition over to oclif/core, so this type of breaking change is ok. With that said, we will probably need to document a migration guide from oclif/... to oclif/core. There will be an announcement issue we will be posting soon. I'll update this when we have that.

I will discuss a few quality of life aspects for ESM and Oclif in general.

• support file URL for runtime entry point
• add flush to main export of @oclif/core

Both of those changes seem fine. I didn't see them in your fork though (PR nodejs/modules#144). Do you plan on adding those?

Edit: Removed a previous suggestion as it creates syntax errors.

from core.

@amphro Excellent to hear that this effort is going to get into Oclif v2! Thanks for indicating general approval w/ the PR as that was quite a surprise. I'm currently finishing up major refactoring / final polish to my Oclif v2 middleware efforts and should have that complete early next week and can get started on addressing the concerns in the PR and assist on updating other Oclif plugins as they become available for review along with documentation efforts.

The initial fork / PR contains just the core changes necessary to support ESM. I figured it would be best to submit changes in a sequence of a few steps / additional PRs, so I avoided adding the quality of life changes and potential changes for the help subsystem going fully asynchronous. I have already prototyped the help subsystem changes in my middleware. I will get to writing responses to the comments you raised in the PR by early next week and I should be freed up at that time to continue the process. Let me know if the basic strategy of breaking things down into 2 PRs makes sense.

from core.

@amphro Just wanted to drop a note that I'm still around. I actually will be finished with the coding queue I've been working through this week for sure. Looking forward to getting to Oclif v2 ESM PR and such soon.

from core.

@amphro ... Alright.. I'm ready to get the process rolling and generally available on a more steady basis through the Oclif v2 launch. I have responded to your initial review in the current PR. As mentioned I think the best way to proceed is for me to make a new feature branch that includes your suggestions and also includes the quality of life changes I mentioned above along with ./src/help/index.ts / help subsystem made fully asynchronous.

from core.

Sounds good @typhonrt - I closed out the PR I created with some additional comments.

If possible, try to have several smaller PRs that are focused: ESM, help, quality of life. I think it will make the PR reviews easier. I understand that may be more challenging as you have already made a bunch of changes on the main branch of your fork. I don't want to waste your time so if it will take too much, then one PR is fine.

from core.

No worries, sounds like a plan.. That was probably my 4th attempt at adding ESM between v1 and v2 so 5th attempt is the charm and should be done soon; I have a reasonable understanding of the core. :D

from core.

@amphro The first PR (#160) which just integrates ESM loading for commands and hooks is available. The second PR to integrate ESM loading for help classes depends on it (ModuleLoader), so I'll start on that after the first PR is merged. I'll hold off on the third QOL PR as well and just do them in order.

from core.

@amphro Indeed there is a 2nd PR (#163) which significantly strengthens ModuleLoader and fixes an oversight from the initial PR (#160) and adds the ability to load all ESM plugins, mixed ESM w/ CJS, mixed CJS w/ ESM, and continuing support for TS loading. The ModuleLoader unit tests are improved, but more importantly there are now full Oclif config / runtime tests which will catch issues for ESM support during runtime. Had the runtime tests been added previously I wouldn't be making a second PR for core ESM support. These new runtime tests also show the versatility of ModuleLoader and mixed ESM / CJS plugin support.

from core.

@amphro Thanks for the nodejs/modules#163 merge. I now have the asynchronous help subsystem PR nodejs/modules#165 available for review. Once this passes muster and is merged I can complete the final quality of life PR for ESM CLIs regarding invoking Oclif itself.

from core.

nodejs/modules#165 looked good and I had no feedback, so it is merged. I wanted to make some of the quality of life improvements too but I have been holding off until this issue is closed. Let me know when your file PR is up for review.

Once this is issue is closed, we will also be making some larger updates to the default help class which we think is much better UX for CLI help.

from core.

@amphro Alrighty.. The ESM quality of life PR nodejs/modules#166 has been submitted for review. This rounds out all of the changes required to provide ace support for ESM via Oclif v2. Exciting days ahead. I'll be available to assist in any updates required to other Oclif plugins as they are made publicly available if necessary. I'd be glad to consult as well on documentation suggestions. I'll be updating all of my demo repos shortly once the latest PR is merged.

Thanks for the opportunity to add ESM support to Oclif. I believe this will definitely give the edge for v2 when it comes to choice of CLI tooling and future proof it for the coming ESM wave on Node.

from core.

@typhonrt LETS GOOOO, thank you so much for your hard work!

from core.

Hey @typhonrt,
with your description it was easy to create a esm oclif thanks for that.

No I want to combine with subcommands introduced here:
oclif/oclif#186 (comment)

Therefor i cloned the hello-world example and transformed it into ES-Module
https://github.com/phibar/hello-world

Unfortunately I missed something...
because with those nested commands, oclif falls back to a cjs require and usage of nested commands failed.

Expected: (hello-world example from linked issuecomment)

Error: (hello-world from phi bar/hello-world)

the topic is resolved but the command isn't

The main difference is that "hello" command is now inside hello folder in index file.

Perhaps you know what is going wrong here, would be great if you can give me a hint what to change, or where to investigate.

from core.