Skip to content

RFC: Standard package output folders #17298

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.

Sign up for GitHub

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

Closed
dzearing wants to merge 8 commits into from
Closed

RFC: Standard package output folders #17298

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4d9acbc
Initial rfc
dzearing Mar 5, 2021
3526bb3
tweaks
dzearing Mar 5, 2021
e3e6ca9
Update 02_standard_package_output_folders.md
dzearing Mar 5, 2021
0f94c61
Update 02_standard_package_output_folders.md
dzearing Mar 5, 2021
2addfe5
Update 02_standard_package_output_folders.md
dzearing Mar 5, 2021
96bb7bc
Update 02_standard_package_output_folders.md
dzearing Mar 5, 2021
38b7c81
Update 02_standard_package_output_folders.md
dzearing Mar 5, 2021
f0a62f4
Update 02_standard_package_output_folders.md
dzearing Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions rfcs/build-system/02_standard_package_output_folders.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# RFC: Standard package output folders

---

_List contributors to the proposal: @dzearing_

## Summary

There are multiple javascript formats we may include in our npm packages: `commonjs`, `esm`, and `amd`, in addition to static assets like pre-made bundles and images.

We should adopt a predictable folder structure within the published npm package for these formats. That way consumers of our packages can know exactly where things are and which folders have what things in as few steps as possible.

This is one of those small details that other partners will emulate and adopt. If we can be consistent, many packages exported by our partners will likely follow guidance because it is really minutia, but ends up adding to more predictability at scale.

## Problem statement

Today, `lib` contains esm JavaScript output, except for node-only packages which will drop CommonJS modules in them. This obfuscates what `lib` actually contains. We should adopt standards which add clarity to what output format is used.

## Detailed Design or Proposal

We've historically used the following standard output folders in a published JavaScript package (only applicable output folders would be present):

- `/lib` - esm (as we've had for a long time)
- `/lib-commonjs` - commonjs (only needed while Node <= 13.2.0 is supported)
- `/lib-amd` - amd (hopefully we can drop someday)
- `/dist` - bundles and static content
Comment on lines +23 to +26
Copy link
Copy Markdown
Member

@layershifter layershifter Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My personal preference is to have lib-esm or lib-es as it obviously says what is in a directory.

Another thing that we discussed some time ago to have a single dist folder. For example:

  • /dist/esm - esm (as we've had for a long time)
  • /dist/commonjs - commonjs (only needed while Node <= 13.2.0 is supported)
  • /dist/amd - amd (hopefully we can drop someday)
  • /dist/assets - bundles and static content

It will have the same props&cons as an alternative proposal, but it will be easier to ignore a single entry in .gitignore/IDE 🐱

Copy link
Copy Markdown
Member Author

@dzearing dzearing Mar 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to change things, I agree this would be even cleaner

Copy link
Copy Markdown
Member Author

@dzearing dzearing Mar 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I could behind the lib-esm approach. It's more explicit, and doesn't favor a particular output. This means we can add more output flavors in the future (the react-native output formats would be the first which come to mind) without the akwardness of picking one as the "true" output flavor.

My primary concern changing now is to avoid destabilizing partners, as Elizabeth mentioned below. My recommendation would be to stick with the current output folders in the writeup, fix the node packages only to stay inline with other packages (since this was a recent change.)

Then in the converged packages, we could start adopting the dist/* layout now. Later in v9+, change everything to be inline with the dist approach.

Is this reasonable/ideal? Concerns?

Copy link
Copy Markdown
Member

@layershifter layershifter Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My primary concern changing now is to avoid destabilizing partners, as Elizabeth mentioned below. My recommendation would be to stick with the current output folders in the writeup, fix the node packages only to stay inline with other packages (since this was a recent change.)

Then in the converged packages, we could start adopting the dist/* layout now. Later in v9+, change everything to be inline with the dist approach.

Is this reasonable/ideal? Concerns?

No concerns, make sense for me 👍

Should we update a proposed solution to include v8 (as it stays currently) and converged proposal (for example, /dist layout, or another proposal) in this RFC? Or have an another one?

Copy link
Copy Markdown
Member

@ecraig12345 ecraig12345 Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's simple enough that we could cover both v8 and converged in this RFC.

Though the part we didn't totally decide on for v8 is whether lib must be ESM for all packages, even ones where in reality we only need commonjs.


If this seems reasonable, we should adopt and stay consistent to stay predictable to consumers. If a library needs to output CommonJS, it should be output within the `lib-commonjs` folder.

A library like `react-button` which might be consumed by Node and bundlers likely will need both ESM and CommonJS, until Node 13.2.0 or greater is the minimum requirement. Its output will have both a `lib` and `lib-commonjs` folder.

Once 13.2.0 becomes the minimum Node requirement, there is little reason to build CommonJS at all, and I anticipate we'll be removing that folder.

### Pros and Cons

#### Pros

- Almost no difference from what we currently have - only the `build:commonjs-only` task needs to be modified to output to `lib-commonjs`.
- Our partners won't really notice changes here at all, since this has been the convention we've used for a long time for nearly all the packages currently being consumed.
- ESM is becoming the standard that all platforms will snap to, hence why `lib` and not `lib-esm`.
- Changes in output won't change the folder structure
- Unchanging folder structures mean that full builds are required less
- End users can predict which folders contain what format
- Scalable. If we need other formats in our JS packages, we can update add more formats.
- `CommonJS` was the standard of the past. More and more, modern node libraries will be moving to ESM as Node 13.2.0+ now supports it. I anticipate this format to phase out over the next few years as AMD has.

#### Cons

- Potentially `esm` might be changed as the "standard" library format in the future, which would require us to adjust what `lib` means.

### Alternatives

Many libraries in OSS have kept `lib` representing CommonJS and have added `es` to the mix as the folder for containing esm. This is very likely because they've existed for a while and didn't want to change their existing folder structure. We could also consider this convention. React itself avoids `lib` and explicitly uses `cjs` to indicate the output format. (It also has no ESM flavor.)

* `/cjs` - CommonJS
* `/es` - esm
* `/amd` - amd
* `/dist` - statics and bundles

#### Pros:

* Follows a few more OSS library conventions (though there isn't consistency here)
* Won't have to revisit if the JavaScript flavor of the month changes later

#### Cons:

* A bigger shift than the original proposal - will impact nearly all of our packages
* Definitely more work to clean things up to this format

### How do other major packages organize their output?

"Standards" haven't really emerged other than `dist` being the output folder. There is a commonality of using `es` as the esm folder. Here's a small sampling:

* @angular/core - Uses `bundles` for output which includes a UMD bundle for commonjs support, and `fesm2015` for esm.
* @apollo/client - No folders; `index.js` at root is esm while `index.cjs.js` is CommonJS.
* @material-ui/core - Uses a root `index.js` rollup entry for commonjs and `es` folder for esm.
* antd - Uses `es` for esm, `lib` for commonjs.
* immer - Uses only `dist` and rolls up a separate `esm` bundle.
* react - Commonjs is in `cjs`, there's also a `umd` folder for bundles.
* redux - Uses `es` for esm, `lib` for commonjs.
* styled-components - Only `dist` with `s-c.esm.js` and `s-c.cjs.js`. They also emit a react-native `s-c.native.js` flavor.
Copy link
Copy Markdown
Contributor

@Hotell Hotell Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

important thing to answer - is this RFC for both legacy and converged?

I'd follow google/preact crew way of shipping libraries. They have plenty of experience in both building smallest possible libraries and way of shipping those to consumers.

For inspiration -> https://github.com/developit/microbundle

Convergence output:

  • one file rollup bundles (3 types only)
  • shipping source is discouraged as you're allowing consumers to use private API's (I understand this was allowed in v0/v7,8 - I recommend to fix this in v9)
"main": "./dist/foo.umd.js", // legacy UMD output (for Node & CDN use)
"module": "./dist/foo.module.js", // legacy ES Modules output (for bundlers)
"exports": "./dist/foo.modern.js", // modern ES2017 output

Pros:

  • smallest possible JS stored in registry === faster installs, less "garbage" in node modules
  • rollup-ed files faster to parse by bundlers instead of parsing and bundling N modules
  • esbuild can be enabled without any further modification
  • no exposure of private APIs
  • only 1 output instead of 2 for "legacy consumers" (cjs/amd -> umd)

Cons:

  • might not work for legacy (v0,7,8) as they consume private apis/nested paths

Copy link
Copy Markdown
Contributor

@Hotell Hotell Mar 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regarding dist/, I definitely agree with following suggestion/change:

I briefly mentioned in on of my PR's regarding better DX - TS path aliases, but it got lost within changes...

I'd recommend to have only one dist, coverage(out of topic) folder in root of monorepo

/coverage
  /packages/
     /react-button
/dist
   /packages/
     /react-button
       | - index.umd.js
       | - index.module.js
       | - index.modern.js
       | - index.d.ts
       | - Readme.md
       | - License.md
       | - package.json  // normalized package.json - contains only important data (deps/peerDeps/license,etc)- no scripts/devDeps and other "baggage"
/packages
   /react-button
/apps
  /todo-app

Pros:

  • source code is not being mismatched with generated code
  • all generated assets live under one roof
  • faster cleanup
  • ease of processing

Cons:

  • might not work for v0,7,8 as they are not leveraging TS path aliases - additional work would be needed

Copy link
Copy Markdown
Member

@ecraig12345 ecraig12345 Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

important thing to answer - is this RFC for both legacy and converged?

Any nontrivial changes will have to be for converged only (the RFC needs to be updated to mention this). #17298 (comment)

Copy link
Copy Markdown
Member Author

@dzearing dzearing Apr 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hotell I think 1 dist folder with rollup bundles makes tons of sense for small libraries. With converged packages where they're more single purposed, for example. It is also a very good tool for avoiding deeply nested imports; and as you mentioned, we would probably discourage including build output completely. (There are a few scenarios where it may be useful, like documentation.) Would be nice to roll up the typings as well.

It breaks down in monolithic suite packages that re-export a lot of things, because of a variety of reasons. More to parse unnecessarily, more prone to side effect problems, bundle graph doesn't show enough detail.

So I could get behind this for the converged smaller-purpose packages.

Copy link
Copy Markdown
Member Author

@dzearing dzearing Apr 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this:

Legacy: we keep the same. lib (esm), lib-commonjs (commonjs), amd (amd)

Converged:

dist/
   react-button.esm.js
   react-button.d.ts
   react-button.commonjs.js
   react-button.umd.js

@layershifter @ecraig12345 @JustSlone thoughts?


## Discarded Solutions

The current solution of using `lib` for either CommonJS or ESM depending on which platforms are (currently) supported should be replaced with something that doesn't mean different things in different contexts.