Question: external editor

[armano2]

I have been reviewing code base, and i'm trying to figure out what is purpose of editor package as prompt for it is implemented directly in inquirer because of that, even if user does not plan to use this feature is required to include iconv-lite

iconv-lite is a huge package and in comparison to library size its~40% of this project

Inquirer.js/packages/inquirer/package.json

Line 44 in ca70e03

"external-editor": "^3.0.3",

Inquirer.js/packages/inquirer/lib/prompts/editor.js

Line 12 in ca70e03

class EditorPrompt extends Base {

shouldn't this be moved to to editor package?

[Mikescops]

I'm raising this issue because I'm also concerned about the size of this dependencies.

Here is a look at my esbuild meta info and it's something that is hard to compress.

[SBoudrias]

@Mikescops all prompts are now available as standalone packages. If size is a concern, you can use those directly and not have the editor prompt in your dependencies.

e.g https://www.npmjs.com/search?q=%40inquirer

[Mikescops]

@SBoudrias Thanks for the answer, I'll follow this recommandation, but what bugs me is that the main package is supposed to be an ESM so the tree shaking should not take into account the editor prompt and its dependencies if I don't use it.
Maybe there is still an issue to solve here right?

[SBoudrias]

@Mikescops which package are you using today? inquirer or @inquirer/prompts?

In the way inquirer is setup, there's no direct prompts imports. So it for sure won't get tree shaken.

It should in the new packages, you can see the package content on npm - it does ship pure ESM. But it's a dual module format package, so make sure your build chain doesn't end up importing the CJS versions.

[Mikescops]

@SBoudrias I'm using @inquirer/prompts, when i look at the map everything seems to be imported in the final build.

In my code i only have this import { confirm, input, password, select } from '@inquirer/prompts';

Here is the chain that is happening:

[SBoudrias]

Are you sure your build tool should/would tree-shake that? Here it just shows it follows all import, so it doesn't get treeshake. Not sure where it's coming from... Does it require the whole tree to be ESM to treeshake?

[Mikescops]

I'm using esbuild which is a well known bundler, and they have tree shaking: https://esbuild.github.io/api/#tree-shaking

From their documentation:

Note that esbuild's tree shaking implementation relies on the use of ECMAScript module import and export statements. It does not work with CommonJS modules. Many packages on npm include both formats and esbuild tries to pick the format that works with tree shaking by default.

I'm trying to poc around to see if I miss something obvious.

[Mikescops]

Well, I've played with different things but I can't figure out what's going on. I'll use the initial solution of only adding the sub dependencies I need.

[Mikescops]

@SBoudrias I figured out overnight what's going on.

I was reading a nice blog post about how tree shaking works: https://blog.jetbridge.com/mastering-javascript-tree-shaking/

And I read the part around side effects, that is also mentioned in the documentation of esbuild: https://esbuild.github.io/api/#tree-shaking-and-side-effects

I didn't understood at the beginning what it exactly meant but this quote helped:

So I gave it a try with the editor module:

{
  "name": "@inquirer/editor",
  "version": "2.1.8",
  "description": "Inquirer multiline editor prompt",
  "main": "./dist/cjs/index.js",
  "typings": "./dist/cjs/types/index.d.ts",
  "files": [
    "dist/**/*"
  ],
  "sideEffects": false,
  }

And 🎉

• the iconv-lite dependency is no more required.

I assumed here that your package doesn't have any "side effects". If that's true and if you agree, I think marking your sub dependencies as "sideEffects": false, would be a great addition for anyone like me using tree shaking 👍

[SBoudrias]

Thanks, I'll add that field in!

[Mikescops]

Merci @SBoudrias ! 💪