Export `run` function as a streamlined but flexible main entrypoint #10

timriley · 2023-10-10T12:42:41Z

Introduce a new assets.run entrypoint function that is:

Streamlined enough for us to generate into every new Hanami app
But flexible enough for users to provide arbitrary esbuild customisations

Here's what a default config/assets.mjs file would look like using this API:

import * as assets from "hanami-assets";

await assets.run();

And here's how it would look to add a custom esbuild plugin:

import * as assets from "hanami-assets";
import postcss from "esbuild-postcss";

await assets.run({
  esbuildOptionsFn: (args, esbuildOptions) => {
    const plugins = [...esbuildOptions.plugins, postcss()];

    return {
      ...esbuildOptions,
      plugins,
    };
  },
});

I've tested this exact config above in a real Hanami app and can confirm it actually works in terms of activating esbuild plugins.

In terms of code changes here, they are fairly wide-ranging. Sorry for the messy diff.

Let me guide you through the new landscape, however:

index.ts: defines the run function that serves as the main entry point, as shown above in those config/assets.mjs examples. This accepts an object of options so that they can all be optional.
args.ts: a new file that defines a proper interface and return object for the args that we parse when invoking node config/assets.mjs inside Hanami apps. This makes sure t things are nicely typed when passing an esbuildOptionsFn per the second example above.
esbuild.ts: defines our standard esbuild options, as used by the run function above. Most of the code here has been simply moved into a new location, otherwise left unchanged, aside from the new buildOptions and watchOptions functions that return the combined set of options for the two modes that we use.
esbuild-plugin.ts: defines our esbuild plugin. I've barely touched this file, except for a few light renames of its exports.

That really sums it up! If you spend most of your time checking out index.ts, args.ts and skimming esbuild.ts, you'll be up to speed with the changes.

There are a few other things I took care of while I was here:

Removed the config indicating that this package exposes an executable. Running via a config/assets.mjs script is now the one and only way to use this package.
Use Prettier for JS file linting/auto-formatting. This introduced a fair bit of churn, but I needed it because I wasn't sure how I should be formatting my TypeScript. 😆 And now's as good a time as any to get this into place.
Update TypeScript compilerOptions to use "module": "NodeNext". Per their relevant docs:

You very likely want "nodenext" for modern node projects.

For full context of how this will be used within a full Hanami app, see hanami/cli#109.

… function

Gosh, this tripped me up for like a good couple hours in total

This will always have a plugins, so it's better to reflect that. This also makes it easier to use in `config/assets.mjs` files.

The intention is that this is run via a script only now.

Go with a shorter name for tidiness

timriley · 2023-10-11T12:23:17Z

src/esbuild.ts

+const findEntryPoints = (root: string): Record<string, string> => {
+  const result: Record<string, string> = {};
+
+  // TODO: should this be done explicitly within the root?


I added this TODO. It looks like we're finding files via these globs, but without actually specifying the root from which it should be operating. This is probably only working because globSync is implicitly working off the cwd?

timriley · 2023-10-11T12:23:46Z

src/esbuild.ts

+  return result;
+};
+
+// TODO: feels like this really should be passed a root too, to become the cwd for globSync


This was the other TODO I added here. The one other place in this file where we were doing file operations without providing an explicit root.

@timriley IIRC, by adding the absolute path, Esbuild will report the absolute path in the metadata, so it's a bit cumbersome to work lately with them in the context of manifest generation.

Ah, this is useful to know, thanks @jodosha!

The only downside of the current arrangement is that it's kind of contingent of the cwd also equalling the app root. In 99% of usage, I don't expect these things will diverge, but the fact that we're allowing a root: to be provided to assets.run means that some of these globs that reply on the cwd might start to fail.

However, I'm comfortable enough with us leaving this as a known gap for the moment. We can come back to solidify this aspect later.

And perhaps we don't document the assets.run root: option as "public" for now, too.

timriley · 2023-10-11T12:24:25Z

src/esbuild.ts

+  }
+};
+
+// TODO: reuse the logic between these two methods below


I also added this TODO, but I'd rather leave it for another PR to refactor these. I left these lists of options mostly unchanged for the sake of continuity.

timriley · 2023-10-11T12:25:43Z

jest.config.ts

+import type { JestConfigWithTsJest } from "ts-jest";
+
+const config: JestConfigWithTsJest = {
+  testEnvironment: "node",
+  testMatch: ["**/test/**/*.test.ts"],
+  // See https://kulshekhar.github.io/ts-jest/docs/guides/esm-support#esm-presets for below
+  preset: "ts-jest/presets/default-esm",
+  moduleNameMapper: {
+    "^(\\.{1,2}/.*)\\.js$": "$1",
+  },
+  transform: {
+    "^.+\\.tsx?$": [
+      "ts-jest",
+      {
+        useESM: true,
+      },
+    ],
+  },
+};
+
+export default config;


This whole bit of boilerplate config was required after I changed the package.json to include "type": "module" and opt us into modern day ES Modules.

timriley · 2023-10-11T12:41:27Z

Lastly, apologies for the messy commit history here. This took a few twists and turns (and some of the usual "Tim figures out how to JavaScript again") before I got everything settled.

This is going to be squashed on merge anyway, and I'll make sure there's a good single commit message then. In the meantime, please see my overview in the PR description for the best way of inspecting the changes in this PR.

jodosha

@timriley On the other day you were "unreasonably excited" about these changes. Now I understand why: they are magnificent! They're taking this code base to the next level both in what they offer to end users, but also in quality of the internal code. Well done! 👏

Please check my inline comment about "bin" in package.json. 🙂

jodosha · 2023-10-12T07:53:31Z

package.json

@@ -1,12 +1,13 @@
 {
  "name": "hanami-assets",
  "version": "2.1.0-beta.2",
-  "description": "Hanami Assets management via Esbuild",
+  "type": "module",
+  "description": "Hanami assets management via Esbuild",
  "bin": {


@timriley We must remove "bin" as we don't have the executable anymore. 🙂

Ooh yes, I thought I removed it and even mentioned in the PR description, but clearly did not 😆

package.json

jodosha · 2023-10-12T08:00:51Z

src/esbuild.ts

+  return result;
+};
+
+// TODO: feels like this really should be passed a root too, to become the cwd for globSync


@timriley IIRC, by adding the absolute path, Esbuild will report the absolute path in the metadata, so it's a bit cumbersome to work lately with them in the context of manifest generation.

Prepare for our updated approach to assets chiefly introduced with hanami/assets-js#10. Update `hanami new`: - Generate a new `config/assets.mjs` that will be needed for our new way of running the assets commands (see hanami/assets-js#10) - - Generate a leaner `package.json`, including an `"assets"` entry in the `"scripts"` section, referencing the above `config/assets.mjs` file. Update the `hanami assets compile` and `hanami assets watch` commands: - Expect to run assets via `npm run assets`, as generated in the new `package.json` above. - Have these use a single hanami-assets setting for running these commands: `package_manager_run_command` (introduced in hanami/assets#131), which defaults to `npm run`.

timriley added 9 commits October 8, 2023 23:02

Move loader and entryPoints into esbuild-options module

864b9f9

Move externalDirectories to esbuild-options

829552b

Make buildOptions a function

6d83c52

Move watchOptions to function

9cd6213

Rename esbuild-options to esbuild, export parseArgs and touchManifest…

43aaf26

… function

Some module business I don’t really understand

4c958eb

Get everything working

67376bd

Format everything with Prettier

510cbf3

Lint with Prettier in CI

de5d5d2

timriley force-pushed the more-exports branch from e5c84b3 to e9fb1c6 Compare October 10, 2023 12:44

Don’t lint markdown files with Prettier

f003370

timriley force-pushed the more-exports branch from e9fb1c6 to f003370 Compare October 10, 2023 12:45

timriley added 11 commits October 11, 2023 00:06

Fix “main” entry in package.json

7b42c71

Gosh, this tripped me up for like a good couple hours in total

Update tests again

8c2639d

Use an options object for assets.run

61be110

Allow no optiopns to be provided, tweak arg names for esbuildOptionsFn

7bf8c9e

Use a specific interface for our esbuild options

6d34d18

This will always have a plugins, so it's better to reflect that. This also makes it easier to use in `config/assets.mjs` files.

Don’t make hanami-assets.js executable any more

dff244e

Rename main file to index and remove package bin entry

c729cd1

The intention is that this is run via a script only now.

Run prettier again

2540d6d

Rename hanami-esbuild-plugin -> esbuild-plugin

42b2d8e

Go with a shorter name for tidiness

A bit more light renaming

f5c2692

Remove TODO

78889c3

timriley marked this pull request as ready for review October 11, 2023 12:20

timriley requested a review from jodosha October 11, 2023 12:20

timriley commented Oct 11, 2023

View reviewed changes

timriley mentioned this pull request Oct 11, 2023

Generate and install new assets arrangement hanami/cli#109

Merged

timriley self-assigned this Oct 11, 2023

jodosha approved these changes Oct 12, 2023

View reviewed changes

jodosha added this to the v2.0.0 milestone Oct 12, 2023

timriley added 2 commits October 12, 2023 20:08

Remove bin entry from package.json

2cee8cf

Remove ts-node

ec1131d

timriley merged commit 868d43e into main Oct 12, 2023
2 checks passed

timriley deleted the more-exports branch October 12, 2023 09:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export `run` function as a streamlined but flexible main entrypoint #10

Export `run` function as a streamlined but flexible main entrypoint #10

timriley commented Oct 10, 2023 •

edited

Loading

timriley Oct 11, 2023

timriley Oct 11, 2023

jodosha Oct 12, 2023

timriley Oct 12, 2023

timriley Oct 11, 2023

timriley Oct 11, 2023 •

edited

Loading

timriley commented Oct 11, 2023

jodosha left a comment •

edited

Loading

jodosha Oct 12, 2023

timriley Oct 12, 2023

jodosha Oct 12, 2023

Export run function as a streamlined but flexible main entrypoint #10

Export run function as a streamlined but flexible main entrypoint #10

Conversation

timriley commented Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timriley Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

timriley commented Oct 11, 2023

jodosha left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Export `run` function as a streamlined but flexible main entrypoint #10

Export `run` function as a streamlined but flexible main entrypoint #10

timriley commented Oct 10, 2023 •

edited

Loading

timriley Oct 11, 2023 •

edited

Loading

jodosha left a comment •

edited

Loading