Allow references to files outside js/ and css/ dirs

timriley commented 8 months ago

Stop marking the directories aside from js/ and css/ as "external". This allows the files inside those other directories to be referenced from within JS/CSS files and properly bundled by esbuild.

This incorporates @krzykamil's (excellent!) suggestion in https://github.com/hanami/assets-js/pull/27, which uses an onLoad callback to track referenced files so that we can exclude them from separate handling when we copy over the non-referenced asset files.

Specific changes:

Stop marking any directories as external (in esbuild.js)
Move findExternalDirectories from esbuild.js to esbuild-plugin.js as extraAssetDirs (a more appropriate name now that nothing is marked as external), and call this directly when determining which directories we should manually copy via processAssetDirectory.
Introduce an onLoad callback to esbuild-plugin.js and use this to track any files that esbuild itself loads.
Use this to skip already-loaded files inside processAssetDirectory, which means there will be no double-processing of the files that are referenced from JS/CSS.
Then when handling esbuild's outputs (i.e. all the loaded files) to include in the manifest, make sure to use the output's original input filename for the manifest key, to avoid collisions and to keep those manifest keys consistent with non-loaded files that are copied across as part of processAssetDirectory.
Plus a range of other refactors/tidyings

Thanks also to @svoop for the great initial bug report (in #24 as well as the ensuing forum conversation)!

Fixes #24.

krzykamil commented 8 months ago

https://github.com/hanami/assets-js/pull/27 my take on the TODO @timriley

timriley commented 8 months ago

@krzykamil your idea is brilliant! I've just incorporated it into this PR. Could you please test things again on your app, using this branch?

krzykamil commented 8 months ago

It looks fine, fonts are not duplicated and everything is correctly nested in my public/assets/_main

timriley commented 8 months ago

Just noting that using this branch with an app with slice assets fails to include the slice's JS/CSS files in the manifest. See this forum post for details.

This is a bug and would be a regression. I'll address this before wrapping up this PR (and hopefully capture this in a test somehow).

hanami / assets-js

Allow references to files outside js/ and css/ dirs #26