lumeland / lume

🔥 Static site generator for Deno 🦕
https://lume.land
MIT License
1.86k stars 85 forks source link

Can't enable code splitting in esbuild #323

Closed DrSensor closed 1 year ago

DrSensor commented 1 year ago

Version

1.31.1

Platform

Linux

What steps will reproduce the bug?

Here is my _config.ts

import esbuild from "lume/plugins/esbuild.ts";
import minify_html from "lume/plugins/minify_html.ts";

import lume from "lume/mod.ts";

export default lume()
  .ignore("lib")
  .use(esbuild({
    options: {
      splitting: true,
      mangleProps: /^_/,
    },
  }))
  .use(minify_html());

How often does it reproduce? Is there a required condition?

When splitting option enabled in esbuild

What is the expected behavior?

For example, I have this project structure:

lib
└── utils.ts

elements
├── render-scope.ts
└── page-link.ts

index.html

where both render-scope.ts and page-link.ts depend on lib/utils.ts.

If I run it with esbuild CLI, the result would look like this:

$ esbuild elements/*.ts --bundle --splitting --outdir=_site/elements --format=esm

  _site/elements/chunk-YLOKRXMB.js  5.0kb
  _site/elements/page-link.js       2.0kb
  _site/elements/render-scope.js    1.6kb

⚡ Done in 9ms

What do you see instead?

$ deno task build

Task build deno task lume
Task lume echo "import 'lume/task.ts'" | deno run --unstable --allow-read --allow-run --lock=lock.json -
Loading config file _config.ts

✘ [ERROR] Must use "outdir" when code splitting is enabled

✘ [ERROR] Must use "outdir" when code splitting is enabled

✘ [ERROR] Must use "outdir" when code splitting is enabled

✘ [ERROR] Must use "outdir" when code splitting is enabled

Error: Error processing page
- page: /demo/counter/module.js
- processor:
    at (https://deno.land/x/lume@v1.13.1/core/processors.ts:44:19)
    at async concurrent (https://deno.land/x/lume@v1.13.1/core/utils.ts:175:3)
    at async Processors.run (https://deno.land/x/lume@v1.13.1/core/processors.ts:34:7)
    at async Site.#buildPages (https://deno.land/x/lume@v1.13.1/core/site.ts:610:5)
    at async Site.build (https://deno.land/x/lume@v1.13.1/core/site.ts:494:9)
    at async build (https://deno.land/x/lume@v1.13.1/cli/build.ts:36:3)
    at async Command.execute (https://deno.land/x/cliffy@v0.25.4/command/command.ts:1790:7)
    at async (https://deno.land/x/lume@v1.13.1/cli.ts:147:3)

Caused by Error: Build failed with 1 error:
error: Must use "outdir" when code splitting is enabled
    at failureErrorWithLog (https://deno.land/x/esbuild@v0.15.14/mod.js:1546:15)
    at (https://deno.land/x/esbuild@v0.15.14/mod.js:1004:28)
    at runOnEndCallbacks (https://deno.land/x/esbuild@v0.15.14/mod.js:1418:61)
    at buildResponseToResult (https://deno.land/x/esbuild@v0.15.14/mod.js:1002:7)
    at (https://deno.land/x/esbuild@v0.15.14/mod.js:1114:14)
    at responseCallbacks.<computed> (https://deno.land/x/esbuild@v0.15.14/mod.js:651:9)
    at handleIncomingPacket (https://deno.land/x/esbuild@v0.15.14/mod.js:706:9)
    at readFromStdout (https://deno.land/x/esbuild@v0.15.14/mod.js:627:7)
    at (https://deno.land/x/esbuild@v0.15.14/mod.js:1828:11)

I have also try specifying outdir but it conflict with outfile. I guess the plugin dynamically populate the outfile option 🤔

Additional information

No response

oscarotero commented 1 year ago

Ok, it makes sense. esbuild plugin process one file each time. splitting mode requires to process all files at the same time, in order to discover common dependencies. I'll take a look.

oscarotero commented 1 year ago

I have added support for this. Can you test it, please? Update Lume to the latest development version with deno task lume upgrade --dev. Thanks!

DrSensor commented 1 year ago

Alright it works! But it's quite tricky since I need to specify outdir too

< From esbuild docs > If your build contains multiple entry points in separate directories, the directory structure will be replicated into the output directory starting from the [lowest common ancestor](https://en.wikipedia.org/wiki/Lowest_common_ancestor) directory among all input entry point paths. For example, if there are two entry points `src/home/index.ts` and `src/about/index.ts`, the output directory will contain `home/index.js` and `about/index.js`. If you want to customize this behavior, you should change the [outbase directory](https://esbuild.github.io/api/#outbase). I think the plugin can do better by automatically specifying `outdir` to _site/ and `outbase` directory from `entryPoints` 🤔 (just random guessing)
example ```ts // lib/shared.ts export let count = 0 export function increment(element) { element.value = count++ } ``` ```ts // elements/counter-one.ts import * as shared from "../lib/shared.ts" customElements.define("counter-one", class extends HTMLButtonElement { onclick { shared.increment(this) } }) ``` ```ts // elements/counter-two.ts import * as shared from "../lib/shared.ts" customElements.define("counter-two", class extends HTMLButtonElement { onclick { shared.increment(this); shared.increment(this) } }); ``` without code splitting, `shared.counter` doesn't work as global/shared variable across modules. It can cause clicking `` doesn't increment ``.
oscarotero commented 1 year ago

Okay, I've done some changes to have better defaults when splitting: true. Now you only need:

site.use(esbuild({
  options: {
    splitting: true,
  },
}));

And the dest files will replicate the src directory structure.

I think splitting: true should be default.

This would be confusing if esbuild doen't work in this way by default. Note that code splitting is still work in progress in esbuild.

DrSensor commented 1 year ago

I just notice the new release and when splitting:true it keep the file extensions instead of renaming it to .js

  .use(
    esbuild({
      extensions: [".ts"],
      options: {
        splitting: true,
        mangleProps: /^_/,
        minify: false,
      },
    }),
  )
🔥 /demo/counter/module.js /demo/counter/module.js
🔥 /demo/counter/index.html /demo/counter/index.html
🔥 /lib/render-scope.ts /lib/render-scope.ts
🔥 /lib/std.ts /lib/std.ts
🔥 /chunk-DZLRBH36.js (generated)
oscarotero commented 1 year ago

Ups! Sorry, I'll fix it. Meanwhile, you can use a procesor to fix it:

site.process([".ts"], (page) => page.data.url.replace(/\.ts$/, ".js");
oscarotero commented 1 year ago

Fixed in Lume v1.14.0

DrSensor commented 1 year ago

@oscarotero Bad news, it broke again 😅

$ deno task build

Loading config file file:///run/media/wildan/Projects/Examples/lume-code-splitting/_config.ts

✘ [ERROR] The JSX syntax extension is not currently enabled

    deno:file:///run/media/wildan/Projects/Examples/lume-code-splitting/index.html:1:0:
      1 │ <!DOCTYPE html>
        ╵ ^

  The esbuild loader for this file is currently set to "js" but it must be set to "jsx" to be able
  to parse JSX syntax. You can use "loader: { '.js': 'jsx' }" to do that.

✘ [ERROR] Expected identifier but found "!"

    deno:file:///run/media/wildan/Projects/Examples/lume-code-splitting/index.html:1:1:
      1 │ <!DOCTYPE html>
        ╵  ^

Error: Build failed with 2 errors:
deno:file:///run/media/wildan/Projects/Examples/lume-code-splitting/index.html:1:0: ERROR: The JSX syntax extension is not currently enabled
deno:file:///run/media/wildan/Projects/Examples/lume-code-splitting/index.html:1:1: ERROR: Expected identifier but found "!"
    at failureErrorWithLog (https://deno.land/x/esbuild@v0.15.16/mod.js:1554:15)
    at (https://deno.land/x/esbuild@v0.15.16/mod.js:1012:28)
    at runOnEndCallbacks (https://deno.land/x/esbuild@v0.15.16/mod.js:1426:61)
    at buildResponseToResult (httprun/media/wildans://deno.land/x/esbuild@v0.15.16/mod.js:1010:7)
    at (https://deno.land/x/esbuild@v0.15.16/mod.js:1122:14)
    at responseCallbacks.<computed> (https://deno.land/x/esbuild@v0.15.16/mod.js:659:9)
    at handleIncomingPacket (https://deno.land/x/esbuild@v0.15.16/mod.js:714:9)
    at readFromStdout (https://deno.land/x/esbuild@v0.15.16/mod.js:635:7)
    at (https://deno.land/x/esbuild@v0.15.16/mod.js:1836:11)

Somehow it treats all files as esbuild input 🤔

oscarotero commented 1 year ago

@DrSensor Ops, sorry. I can't believe I miss that!! I just tagged the hotfix v1.14.1 and now it should work fine.