BjornTheProgrammer / bun-plugin-html

A plugin for bun build which allows html entrypoints
MIT License
33 stars 2 forks source link

[Issue] onLoad plugins must return an object with "contents" #6

Closed gtrabanco closed 10 months ago

gtrabanco commented 11 months ago

Captura de Pantalla 2023-11-25 a las 16 19 08

The code to build is here: https://github.com/BalonmanoVetusta/handball-liveshow-spain/blob/fix_build/bin/build.js

I am receiving this error while building with:

Bun: 1.0.14+93714292b bun-plugin-html: 1.3.2

gtrabanco commented 11 months ago

Also important that with the second build for dashboards of NodeCG it creates just an html file called "html" without any extension (maybe due other errors because graphics didn't fail).

Captura de Pantalla 2023-11-25 a las 16 25 00

BjornTheProgrammer commented 10 months ago

@gtrabanco I've fixed the path resolution issue with release v1.3.3, however as for the return values of Bun.build, that will be much harder.

Currently, I've gotten it to semi-work by making outdir undefined and changing all the entrypoints to be the new generated files, however, there are plenty of issues with this approach, which involves the response object lying to the user, which seems to be much worse behavior.

I've investigated the source code of multiple other plugins, looked at the bun source code, and reread the docs, however at this point it doesn't seem like it is possible to just output any file you want (even with the 'file' loader it breaks when supplied an html file).

What would your opinion be of adding this disclaimer, while more alternatives are explored in the future?

Screenshot 2023-11-28 at 5 55 27 PM
gtrabanco commented 10 months ago

I think you must lie in the return... Whatever the final result is a Promise but when you process file I think you should return { content: '', type: 'text'} in the loader.

BjornTheProgrammer commented 10 months ago

Thanks for your reply, however, I think that lying to the user really doesn't have any utility and just causes more confusion.

Also, because I failed to explain it very well here is the issue. Any loader will return a js file ultimately. This file does not need to and shouldn't exist. So to prevent the file from outputting you have two options, one is to set the outdir to undefined, or to throw an error inside the onLoad function.

Here is an example.

build.config.outdir = undefined;

build.onLoad({ filter: /.*/ }, async (args) => {
    console.log(args)
    return {
        contents: '',
        loader: 'file'
    }
});
args = {
  path: "/Users/bjorn/Desktop/bun-plugin-html/test/starting/index.html",
  namespace: "file",
  loader: "js"
}
response = {
  outputs: [
    BuildArtifact (entry-point) {
      path: "./index.js",
      loader: "file",
      kind: "entry-point",
      hash: "14f3a725d2f57c1a",
      Blob (104 bytes) {
        type: "text/javascript;charset=utf-8"
      },
      sourcemap: null
    }
  ],
  success: true,
  logs: []
}

There is a trick I can do for the js files imported by the html file by adding them as entrypoints and returning the compiled code, but to be honest, this is probably slower and more error-prone.

The problem is that the response object from Bun.build will be completely wrong regardless.

That's why I think it's probably better not to lie to the user, and inform them that response objects aren't a thing currently, and state something like this instead bun-plugin-html does not support output information at this time.

Note that the build actually still succeeds despite the error message.

Would you mind elaborating on your use case, such that we could better accommodate it? Is your main issue that you cannot tell if the build actually succeeded using the response object?

gtrabanco commented 10 months ago

You can output just the js files you created. Anyway you are not lying about the files to the user because you are not "speaking" with regular user, you are speaking with other devs that are modifying intentionally how Bun.build works... But this just my opinion in case you want one.

However the important stuff is that if you download my project and use fix_build branch and run: bun build:bun you expect to have two folders dashboards and graphics. You will see a untested but seems to be fine graphics output but the output of dashboards is wrong.

Here is the project: https://github.com/BalonmanoVetusta/handball-liveshow-spain/tree/fix_build

It is expected to create html files for each dashboard panel of nodecg which are declarated in the package.json but it creates just one file with the name html without any extension. Not sure why 😅

You can view my build script code here: https://github.com/BalonmanoVetusta/handball-liveshow-spain/blob/fix_build/bin/build.js

BjornTheProgrammer commented 10 months ago

Thanks for leaving a reply @gtrabanco. I pushed a patch for this a few days ago. Try upgrading to v1.3.3, it should work with this version, and have the correctly generated name for the html file.

Also I'm not opposed to the idea of giving the output if I could see a valid use case for this behavior, since there are some trade offs.

gtrabanco commented 10 months ago

Yes, I updated and tested with 1.3.3 and I got that issue (the html files incorrectly created).

Sorry if I confused you before 😅

BjornTheProgrammer commented 10 months ago

Weird, the current package.json shows v1.3.2.

Screenshot 2023-12-01 at 9 43 52 AM

When I upgrade to v1.3.3, and run the build code the html seems to generate fine

Screenshot 2023-12-01 at 9 51 46 AM
gtrabanco commented 10 months ago

Ohhh yes... I updated but not push...

I have tested without bun install before, like just for changing the version the bun install was done automatically 😅🤦‍♂️🤦‍♂️🤦‍♂️🤦‍♂️🤦‍♂️

I apologize 🙏

BjornTheProgrammer commented 10 months ago

Lmao @gtrabanco, I know that it can be confusing due to how npm install and bun install so I don't mind. Once again thanks for the bug report! I'll keep looking for a solution to the output issue in the future.