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

Add support for esbuild code splitting #12873

Closed

Conversation

BrianLeishman
Copy link

rel: #7499

This only adds support for the "splitting" option. My plan is to get multiple entry points to work after this, but I figured it would be best to implement the new esbuild options in stages.

Splitting being enabled with only one entry point actually does have an effect, depending on the input. I have put together a test repo to demonstrate by building the "milkdown" editor, which creates a few hundred "chunks" that all get imported by the generated entry point.

git clone https://github.com/BrianLeishman/hugo-milkdown-esbuild --recurse-submodules
cd hugo-milkdown-esbuild
git fetch
git checkout feat/code-splitting
npm ci
hugo server

You can also experiment with removing the "splitting true" from layouts/shortcodes/js.html:17 to see that only the single file is generated again and yet the test applications functions the same.

This branch is based off of my branch in my other pr, #12869, so those changes are being included below, but if the first was merged those would go away.

@bep
Copy link
Member

bep commented Sep 22, 2024

I have in progress work related to this in #12641, so this and related PRs need to wait.

@BrianLeishman
Copy link
Author

Oh this is awesome news!

I'll probably still play with the entrypoint opt as well in the mean time anyway, we have sort of an urgent need out our org. We were using tsickle+closure compiler previously but tsickle has been officially discontinued as of a few year ago now, and it's causing us a world of problems trying to use modern libraries, but esbuild has so far been excellent.

@bep
Copy link
Member

bep commented Sep 22, 2024

I'll probably still play with the entrypoint opt as well in the mean time anyway,

Do as you please, I just wanted to say that it may be a hard sell getting it merged in to Hugo's main branch.

As to this particular PR, I don't think you can add the "code splitting" option without thinking about multiple entry points. And "multiple entry points" needs some coordination/synchronisation to be generally useful.

@BrianLeishman
Copy link
Author

I got the multiple entry points working in another branch, and esbuild is definitely amazing here. Usage is a little clumsy since I was just doing the minimum to get it to function for our needs, but our usage looks like this:

{{ $entrypoints := resources.Match `ts/entrypoints/*.ts` }}
{{ $opts := dict `target` `es2016`  `minify` true  `sourcemap` `linked`  `targetPath` `js/main.js`  `splitting` true  `entrypoints` $entrypoints }}
{{ $js := (index $entrypoints 0) | js.Build $opts | fingerprint }}
{{ $js.Publish }}
{{ range $entrypoints }}
    {{ if eq $entrypoints $.Params.js }}
        {{ $out := printf `%s.js` (strings.TrimPrefix `/ts/entrypoints/` (strings.TrimSuffix (path.Ext .Name) .Name)) }}
        <script src="/js/{{ $out }}" type="module"></script>
    {{ end }}
{{ end }}

Features:

  • entrypoints here accepts a slice of strings (relative from assets, just like inject does), or a slice of resources
  • tmp dir is used every time to build now, and all files in the tmp dir are "published", so building is the same sourcemap or no
  • added support for sourcemap "linked" mode, which adds it's own sitemap comment to all files. Needed here vs "external" because for "external" mode we are only adding the sitemap to the main resource, not all the additional files.
  • This differentiates between which output files are the entrypoint results vs additional chunks, although it doesn't do anything with that info yet

Gotchas:

  • The first resource is used as "the resource" since js.Build needs a single resource, and it has to be published to take effect, but we simply set the target to something that doesn't exist because we're just using the side effects of publishing
  • Because the additional files are just published and aren't resources themselves I can't fingerprint them

I was able to migrate our ~70k line typescript project that used Closure Compiler over entirely yesterday. Since this meets are needs and there is already something in the pipeline I'll likely stop here.

If you would like me to try and polish this and make it more user friendly, have tests, etc, I'd love to try and tackle it! I don't think it would be very difficult to make js.Build accept and returns resources vs just resource, which would make the usage a lot cleaner, and also fix the fingerprinting issue.

@BrianLeishman
Copy link
Author

https://github.com/BrianLeishman/hugo/tree/feat/js--accept-multiple-resources

Just in case you're curious or interested at all, I actually got this to work completely for our environment the way we would have liked.

  • js.Build can take single resource or multiple resources. If given multiple resources it returns a "ResourceCollection" that has a property to loop through all the output resources, and a method to "Get" a resource from the collection by the RelPermalink
  • Resources that are returned are actually transformations of the input resources for each entrypoint. This keeps the chain in-tact for all resources, including the css output files that might come from input resources. Both the js/css outputs would be internal transformations of the related input entrypoint file
  • This means that the resulting js and css can be properly fingerprinted
  • js.Build process is properly cached, using the internal "Resources", which invalidates when the original input resource files are modified, as expected, so js.Build only runs once per instance even if multiple pages include it
  • Code splitting chunks and source maps are being "Publish"ed, so those ones are just available in the public folder, can't retrieve those ones as a resource. That also keeps the chunk names and source map names in-tact so the files referencing them don't need updated. Chunk files have a hash in the name, so that fixes the fingerprinting issue, and sourcemaps don't really need fingerprinting since they are likely only used where caching is disabled in the browser anyway.

Production usage example:

{{ $js := resources.Match `ts/pages/**.ts` }}
    {{ $loader := dict `.svg` `text`  `.ttf` `text` }}
    {{ $opts := dict `target` `es2016`  `minify` (not hugo.IsServer)  `sourcemap` `linked`  `targetPath` `js`  `splitting` true  `loaders` $loader }}
    {{ $js := $js | js.Build $opts }}

    {{ with $js.Get `/js/main.js` }}
        {{ $js := . | fingerprint }}
        <script src="{{ $js.RelPermalink }}" type="module" integrity="{{ $js.Data.Integrity }}"></script>
    {{ end }}

    {{/* multiple js files can be referenced from the front matter */}}
    {{ range .Params.js }}
        {{ $jsName :=  . | printf `/js/%s` }}
        {{ with $js.Get $jsName }}
            {{ $js := . | fingerprint}}
            <script src="{{ $js.RelPermalink }}" type="module" integrity="{{ $js.Data.Integrity }}"></script>
        {{ else }}
            {{ errorf `failed to find %q in %q` . $.File.Path }}
        {{ end }}

        {{ $cssName := strings.TrimSuffix (path.Ext .) . | printf `/js/%s.css` }}
        {{ with $js.Get $cssName }}
            {{ $css := . | fingerprint }}
            <link rel="stylesheet" href="{{ $css.RelPermalink }}" integrity="{{ $css.Data.Integrity }}">
        {{ end }}
    {{ end }}

@bep
Copy link
Member

bep commented Oct 1, 2024

@BrianLeishman great that you have found something that works for you.

I do think, though, that Hugo (or I) needs something else/more, most notably:

  • Better (and more strictly defined) coordination (e.g. what if I want to add a script from a shortcode?)
  • Multiple instances (params) of a given script.
  • And some other minor things.

Also, since I'm most likely ending up maintaining it, I might as well implement this myself. But I appreciate the input.

@bep bep closed this Oct 1, 2024
@BrianLeishman
Copy link
Author

Sounds good! Once a solution exists under master we'll be moving to that

@BrianLeishman BrianLeishman deleted the feat/code-splitting branch October 1, 2024 15:38
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

Successfully merging this pull request may close these issues.

2 participants