Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide multi-module builds #1159

Closed
SBoudrias opened this issue Aug 19, 2022 · 13 comments
Closed

Provide multi-module builds #1159

SBoudrias opened this issue Aug 19, 2022 · 13 comments

Comments

@SBoudrias
Copy link
Owner

If someone want to help, I'd like to configure TS to produce ESM & common.js modules (if possible 馃)

The pure ESM output has been a pain for many users, so I'd like to reduce the amount of support we need to do, and allow people to stay on latest.

@SBoudrias SBoudrias mentioned this issue Aug 19, 2022
@zonemeen
Copy link
Collaborator

zonemeen commented Aug 23, 2022

I recommend trying to use tsup to produce both ESM and common.js modules.

@SBoudrias
Copy link
Owner Author

I tried this out yesterday and there's a major issue I didn't consider... The problem is a lot of packages on npm do not provide cjs builds anymore. So even if I produce esm & cjs modules, they'll fail due to dependencies...

You can see the tentative here https://github.com/SBoudrias/Inquirer.js/tree/multi-module-repo-setup (run yarn ts-node ./tools/setup-packages.ts and yarn lerna run tsc)

@ehaynes99
Copy link

"a lot of packages" are all by the same guy who insists, despite overwhelming evidence to the contrary, that projects no longer need to export commonjs. We'd all be better off finding alternatives for any of them used in node.

Following his advice, you could downgrade versions:

"globby": "^11.1.0",
"chalk": "^4.1.2",
"ansi-escapes": "^4.3.2",
"wrap-ansi": "^7.0.0",

Additionally, since your projects depend on each other, rather than the order from the glob pattern, your setup-packages script should run them in topological order. Otherwise, a lib that depends on core would see that core doesn't have a commonjs export (yet) and fail. At a glance, I think this would be it:

const paths = [
  'packages/type/package.json',
  'packages/testing/package.json',
  'packages/core/package.json',
  'packages/input/package.json',
  'packages/confirm/package.json',
  'packages/checkbox/package.json',
  'packages/editor/package.json',
  'packages/expand/package.json',
  'packages/password/package.json',
  'packages/rawlist/package.json',
  'packages/select/package.json',
];

@LitoMore
Copy link
Sponsor Collaborator

@ehaynes99 I don't think it's a good solution.

We are using TypeScript, we will lose some TypeScript definitions from our dependencies and it's hard to maintain. Then we have to write a bunch of tools, workflows, declarations, and unuseful codes to support CommonJS. Now maintainers feel pain.

@LitoMore
Copy link
Sponsor Collaborator

@ehaynes99 Could you tell why you insist on using CommonJS rather than ESM?

Maybe we can find out another solution to accomplish your requirement.

@SBoudrias
Copy link
Owner Author

@LitoMore honestly I think it's pain from the environment. I ran into multiple issues migrating Inquirer to ESM; it's not easy on Node.js - even though I know it works well with FE babel/webpack/typescript types projects...

But some issues is poor support from Jest (and limitations), and poor and weird support in Typescript for native Node.js ESM (like having to import from non-existing files... 馃う馃徎)

After going through it, I do think the way forward is to build multi-modules output and seeing progressive adoption... The downside to this is it forces a more complicated build process on Node project. Those dependencies listed are mostly small libraries without complex build system.

@ehaynes99
Copy link

I don't insist on it, but it's the topic of this issue, and I think it's a worthwhile goal. Libraries should be as accommodating as possible, not attempt to force project structure upon consumers. To be clear, I'm not saying that maintainers of open source projects must do anything; it's entirely up to each how to prioritize things, but it's going to hurt adoption. It's a real shame when there is a perfectly functional, ubiquitous solution out there that ends up with largely duplicated alternatives not because of some lack of functionality, but simply due to irrelevant tangential problems.

It's still an enormous pain to use ESM with TypeScript. I don't think we actually have the "real" solution yet; it'll undergo additional changes during whatever comes of microsoft/TypeScript#46452 (not least of which the "import from non-existing files" @SBoudrias mentions...). Adoption has been poor at best, and frankly I don't see it improving until it works with only configuration changes that don't change the actual code. Until such time, my $0.02 opinion is that it's premature for libraries to require it. That the issue trackers for the whole ecosystem of popular CLI libraries are filled with issues around this is evidence of this.

For frontend projects, this is often mitigated by bundlers, because the process of flattening all of the dependencies into a single (conceptual) js file removes any need to worry about module resolution at runtime. However, this is not the case for node applications, including those intended to run in the console. TypeScript already replicates the syntactic advantages of ESM, so overall it's a worse developer experience with no tangible benefit. It's not realistic to expect large projects to perform the necessary restructures just to add a prompt to a script or color some console output.

@rxliuli
Copy link

rxliuli commented Oct 20, 2022

I tend to keep esm/cjs dual package support for now, although esm may be preferred. Because production migrations are complex, our production project is a large monorepo with over 100+ modules, which is somewhat easier to batch, but still complex. I've been working with esm only modules since about last year, but didn't really migrate over until this week, which involved a lot of research and compatibility fixes. After the migration of my community monorepo project (30+ modules) was complete, I didn't really start working on projects within the company. If we don't choose vite/esbuild/vitest and stick to webpack/typescript/jest , we probably won't be able to migrate. In fact, we still have some old projects that started 3 years ago that cannot be migrated.
Also, I think it's absurd to impose the concept of esm on the average user and force them to choose one or the other.

@LitoMore
Copy link
Sponsor Collaborator

Here is how to install multi-version of a package:

npm i chalk               # chalk@latest
npm i chalk-4@npm:chalk@4 # chalk@4

Or set aliases to them both:

npm i chalk-lastest@npm:chalk # chalk@latest
npm i chalk-5@npm:chalk@5     # chalk@5
npm i chalk-4@npm:chalk@4     # chalk@4

This may increase our bundle size.

@gabrielgrover
Copy link

I don't insist on it, but it's the topic of this issue, and I think it's a worthwhile goal. Libraries should be as accommodating as possible, not attempt to force project structure upon consumers. To be clear, I'm not saying that maintainers of open source projects must do anything; it's entirely up to each how to prioritize things, but it's going to hurt adoption. It's a real shame when there is a perfectly functional, ubiquitous solution out there that ends up with largely duplicated alternatives not because of some lack of functionality, but simply due to irrelevant tangential problems.

It's still an enormous pain to use ESM with TypeScript. I don't think we actually have the "real" solution yet; it'll undergo additional changes during whatever comes of microsoft/TypeScript#46452 (not least of which the "import from non-existing files" @SBoudrias mentions...). Adoption has been poor at best, and frankly I don't see it improving until it works with only configuration changes that don't change the actual code. Until such time, my $0.02 opinion is that it's premature for libraries to require it. That the issue trackers for the whole ecosystem of popular CLI libraries are filled with issues around this is evidence of this.

For frontend projects, this is often mitigated by bundlers, because the process of flattening all of the dependencies into a single (conceptual) js file removes any need to worry about module resolution at runtime. However, this is not the case for node applications, including those intended to run in the console. TypeScript already replicates the syntactic advantages of ESM, so overall it's a worse developer experience with no tangible benefit. It's not realistic to expect large projects to perform the necessary restructures just to add a prompt to a script or color some console output.

Perfect description of my feelings on this matter. Well done friend!

@gfortaine
Copy link

gfortaine commented Jan 15, 2023

I tried this out yesterday and there's a major issue I didn't consider... The problem is a lot of packages on npm do not provide cjs builds anymore. So even if I produce esm & cjs modules, they'll fail due to dependencies...

You can see the tentative here https://github.com/SBoudrias/Inquirer.js/tree/multi-module-repo-setup (run yarn ts-node ./tools/setup-packages.ts and yarn lerna run tsc)

Here it is 馃帀 cc @SBoudrias @ehaynes99 @LitoMore : #1185

A combination consisting of package aliases + package imports should do the trick 馃殌 (by the way, thx @azu for tsconfig-to-dual-package 馃憤)

{
  "exports": {
    ".": {
      "require": "./lib-cjs/inquirer-default.js",
      "default": "./lib/inquirer.js"
    },
    "./package.json": "./package.json"
  },
  "imports": {
    "#ansi-escapes": {
      "node": {
        "require": "ansi-escapes-4",
        "default": "ansi-escapes"
      }
    },
    "#chalk": {
      "node": {
        "require": "chalk-4",
        "default": "chalk"
      }
    },
    "#cli-cursor": {
      "node": {
        "require": "cli-cursor-3",
        "default": "cli-cursor"
      }
    },
    "#figures": {
      "node": {
        "require": "figures-3",
        "default": "figures"
      }
    },
    "#ora": {
      "node": {
        "require": "ora-5",
        "default": "ora"
      }
    },
    "#string-width": {
      "node": {
        "require": "string-width-4",
        "default": "string-width"
      }
    },
    "#strip-ansi": {
      "node": {
        "require": "strip-ansi-6",
        "default": "strip-ansi"
      }
    },
    "#wrap-ansi": {
      "node": {
        "require": "wrap-ansi-7",
        "default": "wrap-ansi"
      }
    }
  },
  "engines": {
    "node": ">=14.18.0"
  },
  "dependencies": {
    "ansi-escapes": "^6.0.0",
    "ansi-escapes-4": "npm:ansi-escapes@^4.3.2",
    "chalk": "^5.1.2",
    "chalk-4": "npm:chalk@^4.1.2",
    "cli-cursor": "^4.0.0",
    "cli-cursor-3": "npm:cli-cursor@^3.1.0",
    "cli-width": "^4.0.0",
    "external-editor": "^3.0.3",
    "figures": "^5.0.0",
    "figures-3": "npm:figures@^3.2.0",
    "lodash": "^4.17.21",
    "mute-stream": "0.0.8",
    "ora": "^6.1.2",
    "ora-5": "npm:ora@^5.4.1",
    "run-async": "^2.4.0",
    "rxjs": "^7.5.7",
    "string-width": "^5.1.2",
    "string-width-4": "npm:string-width@^4.2.3",
    "strip-ansi": "^7.0.1",
    "strip-ansi-6": "npm:strip-ansi@^6.0.1",
    "through": "^2.3.6",
    "wrap-ansi": "^8.0.1",
    "wrap-ansi-7": "npm:wrap-ansi@^7.0.0"
  }
}

@a7madgamal
Copy link

If I understand the last comment correctly: I don't think "being stuck at an older version" is an acceptable solution to the ESM madness :D
I don't want to be stuck in older versions using hacky complex ways. I want the latest and greatest 馃槄
Please reconsider and just export the lib both ways

@SBoudrias
Copy link
Owner Author

@a7madgamal latest is CJS/ESM compatible. The only one that isn't is inquirer@^9.0.0.

I recommend you migrate to latest (it's not backward compatible, so there's a bit of work; but shouldn't be too bad and can be done progressively.) See #1214

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants