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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing exports #2977

Open
Tracked by #2976
moltar opened this issue Oct 10, 2023 · 5 comments
Open
Tracked by #2976

Fixing exports #2977

moltar opened this issue Oct 10, 2023 · 5 comments

Comments

@moltar
Copy link

moltar commented Oct 10, 2023

By modern npm and Node.js standards, projen does a few things wrong regarding packaging.

  1. There are no package entry points defined in package.json.
    • This essentially means that the "contract" with the consumer is "anything in the package folder", rather than well-defined export paths.
    • This is what CDK gets right, it only exports specific paths (see screenshot)
  2. The entire project tree is exported from / (see screenshot). This needs to be cleaned up. Only contents of lib/ need to be exported, not the root of the projen repo (e.g. projen/javascript not projen/lib/javascript).
  3. Ideally, we should also (or maybe even only) export everything public from just projen, so we don't have import from many sub-paths. E.g. we should be able to do this: import { NodePackage } from 'projen'
  4. Package fails "Are the types wrong?" test, which means it won't work on every system / Node version.

Projen exports:

screenshot-20231010T095812-EyiC5jW7@2x

AWS CDK exports:

screenshot-20231010T102253-CzTjTjyZ@2x


This is a follow up to: #2976 (comment)

@mrgrain
Copy link
Contributor

mrgrain commented Oct 10, 2023

Thank you, this is very helpful and I agree we should start fixing this!

  1. There are no package entry points defined in package.json.
    • This essentially means that the "contract" with the consumer is "anything in the package folder", rather than well-defined export paths.
    • This is what CDK gets right, it only exports specific paths (see screenshot)
  2. The entire project tree is exported from / (see screenshot). This needs to be cleaned up. Only contents of lib/ need to be exported, not the root of the projen repo (e.g. projen/javascript not projen/lib/javascript).

Yes, that needs to be resolved. I think 1) + 2) are the same problem really and due to the missing entrypoint.

  1. Ideally, we should also (or maybe even only) export everything public from just projen, so we don't have import from many sub-paths. E.g. we should be able to do this: import { NodePackage } from 'projen'

Not sure I agree with that. We got a lot of feedback on AWS CDK that submodules exports are the preferred way. It might not even be possible with naming conflicts. And there's also the general disadvantage of using barrel files. 🤔 Why do you think exporting everything at top-level is preferred?

  1. Package fails "Are the types wrong?" test, which means it won't work on every system / Node version.

Looks like this is also mostly due to the missing entrypoint and thus the tool picking up bundled modules. Which I guess you can import today, but really shouldn't. 😓

@moltar
Copy link
Author

moltar commented Oct 10, 2023

Why do you think exporting everything at top-level is preferred?

Fewer import lines. It's not uncommon to have 5 lines importing from projen/lib/**/* for me.

disadvantage of using barrel files

Yes, just saw on HN yesterday a benchmark of barrels - and they do slow things down a lot. But only when not using bundling. If the entire projen is bundled into a single index.js that should not be a problem though. Given that this project isn't meant for the front end, we don't care about tree shaking or other optimizations. Most of the time, the entire file would be loaded once, parsed once and then applied during the single command run.

It might not even be possible with naming conflicts

Yes, thought of that, but didn't investigate. Would be a deal breaker, perhaps, unless, of course, renaming and breaking change is an option when going to v1 ;)

We got a lot of feedback on AWS CDK that submodules exports are the preferred way

But, IMO this could be done both ways?

import { Component, typescript } from 'projen'
import { ... } from 'projen/typescript'

But, ideally I think, at least the high-level components and projects can be exported from the root as well:

import { TypeScriptProject } from 'projen'

Right now we must do the following:

import { TypeScriptProject } from 'projen/lib/typescript'

Which is quite verbose IMO for something so commonly used.

@moltar
Copy link
Author

moltar commented Oct 10, 2023

Yes, that needs to be resolved. I think 1) + 2) are the same problem really and due to the missing entrypoint.

Right, it is about defining entry points.

But those also need to be carefully considered, as they would constitute a public interface. Unless, of course, a wildcard is used exposing everything. But I think this needs to be closed up, and only allow importing from a well-defined export path, like cdk does.


But it can go slightly beyond that also, if we want to have truly universal packages that support both CJS and ESM ecosystems.

Those exports, and bundling, need to support both cases. If we use tsup, all of this can be done out-of-the-box.

This is what a "good" export setup looks like that passes attw checks:

  "main": "./lib/index.js",
  "types": "./lib/index.d.ts",
  "module": "./lib/index.mjs",
  "exports": {
    ".": {
      "require": {
        "types": "./lib/index.d.ts",
        "default": "./lib/index.js"
      },
      "import": {
        "types": "./lib/index.d.mts",
        "default": "./lib/index.mjs"
      }
    }
  },

This structure is generated by tsup, but the package.json entries need to be corrected and added.

@mrgrain
Copy link
Contributor

mrgrain commented Oct 10, 2023

The absolutely only way I recommend everyone to use projen today is:

import { Component, typescript } from 'projen'
new typescript.TypeScriptProject({ /*...*/ });

If that's to verbose you could alias:

import { Component, typescript as ts } from 'projen'
new ts.TypeScriptProject({ /*...*/ });

And with submodule exports, you will have the option to make some trade-offs based on your preferences:

import { Component } from 'projen'
import { TypeScriptProject } from 'projen/typescript'
// or this
import * as ts from 'projen/typescript'

@mrgrain
Copy link
Contributor

mrgrain commented Oct 10, 2023

Yes, just saw on HN yesterday a benchmark of barrels - and they do slow things down a lot. But only when not using bundling. If the entire projen is bundled into a single index.js that should not be a problem though. Given that this project isn't meant for the front end, we don't care about tree shaking or other optimizations. Most of the time, the entire file would be loaded once, parsed once and then applied during the single command run.

Bundling and ESM are another thing to look at - at some point. These can be v2 things, if we adopt a regular release cadence.

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

No branches or pull requests

2 participants