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

馃摝 Packages & Makefile update #130

Open
brianmhunt opened this issue Apr 15, 2021 · 6 comments
Open

馃摝 Packages & Makefile update #130

brianmhunt opened this issue Apr 15, 2021 · 6 comments

Comments

@brianmhunt
Copy link
Member

brianmhunt commented Apr 15, 2021

There are a number of housekeeping issues that need some attention:

  1. updates the package dependencies to the latest versions
  2. look at how leana is used and whether it's still needed in light of support in yarn and/or nom@7 workspaces
  3. look at whether we could use esbuild in place of rollup

We also need to bump the version number to something sane, which my be an "official" number e.g. 4.0.0, even though that would give the impression that there's more "official" stability to the build than is actually warranted.

There are also a number of "bald" packages (e.g. those not in the @tko organization) that need to be marked by npm as deprecated e.g.

@brianmhunt brianmhunt changed the title Packages & Makefile update 馃摝 Packages & Makefile update Apr 23, 2021
@brianmhunt
Copy link
Member Author

brianmhunt commented Apr 26, 2021

I'm moving towards esbuild and nom 7 because esbuild is faster and npm 7 includes workspaces and doesn't require extra utils (i.e. yarn). I'm also removing the Makefile dependencies.

There's quite a bit of experimenting because the nom / node import/export system is rather challenging, from a package developer perspective.

Based on some research I think I've decided I'd like to export both ESM and CommonJS, so I'm compiling dist/index.js for cjs, and dist/index.mjs for ESM, both exposed in package.json:

  "type": "module",
  "exports": {
      "import": "./dist/index.js",
      "require": "./dist/index.js"
  },

The entry point is index.ts (which esbuild picks up), that includes src/index.ts.

Failure: compiling individual files

I had attempted to compile individual (i.e. not-bundled) files into dist/ like this:

src 		:= $(shell find src -name *.js)
dist/index.mjs: $(src) $(peer_src)
	npx esbuild \
		--platform=neutral \
		--log-level=$(log-level) \
		--banner:js="$(banner)" \
		--sourcemap=external \
		--outdir=dist/ \
		$(src)

However this would require some wholesale import file renaming because the extensions need to be explicitly .js (i.e. import ... from './memoization' will fail because it needs to be './memoization.js'. I'm not sure how many files would need renaming, but there look to be over 800 import statements.

@brianmhunt
Copy link
Member Author

brianmhunt commented Apr 26, 2021

Complication: CommonJS + ESM

To keep things simple, at least initially, I'm going to only compile ESM. Since TKO is in (very) low usage, we can aim for future users, and worry about CommonJS when there's demand. In addition to working well with tree-shaking and hopefully modern versions of all the widely used bundlers, it should also work with Deno.

In the future I'd like to revise the exports to be default exports, so we don't need import * from '@tko/...'. What'll be missing from this is the ability to import from node CLI with e.g. require('@tko/...')

@brianmhunt
Copy link
Member Author

Exports

I'm going to rely on the exports field of package.json to disambiguate the type to serve. On the basis that it replicates main and module properties, I'm removing these (though they could be added back in if they prove useful).

@brianmhunt
Copy link
Member Author

Package Priorities

index.js as default ESM

I'm exporting ESM as src/* => dist/* e.g. dist/index.js because esbuild adds .js extensions to the import e.g. import { x } from './array' => './array.js'.

unbundled ESM

The dist is unbundled so that we don't get the double-import problem (e.g. a copy of @tko/utils compiled into every bundle that's imported).

CommonJS

The Common JS is exported bundled as dist/index.cjs

Result

Here's what it looks like:

  "exports": {
    ".": {
      "require": "./dist/index.cjs",
      "import": "./dist/index.js"
    },
    "./helpers/*": "./helpers/*"
  },

Note that the ./helpers/* are exported for test helpers.

@brianmhunt
Copy link
Member Author

Microtasks aside

Noting the queueMicrotask API

This was referenced May 24, 2021
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

1 participant