Support assets in `mel.module` #134

jchavarri · 2024-05-17T14:32:30Z

This PR adds initial support to compile externals like:

external img : string = "default" [@@mel.module "./image.svg"]

Into something like:

let img = "/image-DWDWZCEH.svg"

~~The output file naming follows esbuild default shape and hash function. There is no option to configure anything else for now, we can slowly introduce as needed.~~ Edit: the ppx now support choosing either -bundler esbuild or -bundler webpack. The default is webpack.

I decided to vendor the libraries base32 and xxhash for simplicity, to avoid forcing consumers to pin or download new packages, as they are quite small libraries and the versions that were needed are not available in opam.

jchavarri · 2024-05-23T07:52:35Z

I have updated the PR to support both Webpack and Esbuild, via a new flag -bundler that can be passed to melange.ppx.

For now the generated filenames follow the default name scheme used on each bundler case. We can add more features later on while testing, e.g. file templates like [name][hash][ext] like some bundlers do, or the ability to add prefixes to resulting filenames.

I also have left some failed attempt to produce Rspack hashes commented, as currently rspack doesn't support proper content hashes.

I decided to not include tests against the actual Webpack or Esbuild bundlers hashes because these hashes should not change over time, and it would slow down and complicate the test suite quite a bit.

jchavarri · 2024-05-23T07:57:48Z

packages/melange.ppx/ppx.ml

+    ~doc:
+      "generate paths to assets in mel.module using the file name scheme of \
+       the bundler of choice";
+  Driver.V2.register_transformation ~impl:structure_mapper


Changing to V2 here is what allows to get the ctxt in structure_mapper, which is used later on to get the path to the file being processed (so that we can open it to get its content for the hash).

davesnx · 2024-05-29T09:20:33Z

.github/workflows/ci.yml

@@ -55,6 +55,14 @@ jobs:
          dune-cache: true
          opam-disable-sandboxing: true

+      - name: Install xxhash (ubuntu)
+        if: matrix.os == 'ubuntu-latest'
+        run: sudo apt-get install -y libxxhash-dev


would it mean that all consumers of server-reason-react need to have this installed?

Yes :( Alternatives can be:

put the melange.ppx in a separate package

reimplement the xxhash lib in OCaml

...

and what about adding libxxhash-dev depext?

Good idea 😄 Done in d69747d.

davesnx · 2024-05-29T09:24:38Z

packages/melange.ppx/ppx.ml

+              (* todo: maybe read line by line of buffered for large files *)
+              let ic = open_in asset_path in
+              let n = in_channel_length ic in
+              let s = really_input_string ic n in
+              close_in ic;


I used In_channel.with_open_bin file In_channel.input_all in the past, if you want to avoid having to open/close_in

davesnx · 2024-05-29T09:25:51Z

packages/melange.ppx/ppx.ml

+            (* If it has asset payload (file with extension), calculate hash and replace external *)
+            let name = Builder.pvar ~loc:pval_name.loc pval_name.txt in
+            let path =
+              let asset_path = Filename.(concat (dirname module_path) str) in


Smart, this way we don't care what dune dones

davesnx · 2024-05-29T09:27:17Z

packages/melange.ppx/ppx.ml

+                | Webpack -> Mel_module.Webpack.filename
+                | Esbuild -> Mel_module.Esbuild.filename
+              in
+              let prefix = (* todo: read from config *) "/" in


What's prefix here? The cdn url/etc?

Yeah anything that comes before the file name itself.

davesnx · 2024-05-29T09:34:22Z

packages/melange.ppx/ppx.ml

@@ -401,6 +544,20 @@ module Debug = struct
 end

 let () =
-  Driver.register_transformation ~impl:structure_mapper
+  Driver.add_arg "-bundler"


at first I didn't like the idea of coupling our melange_ppx with a bundler, and I would prefer to keep a --assets-hash-fn where it specifies "xxhash64" or even a format like webpack, but after seeing the complexity it carries... It's perfectly fine to keep it coupled.

Did you thought the same?

It's a bit tricky, because the behavior changes a bit between bundlers:

webpack is the most flexible of those i've investigated, allowing to use any kind of hash function and any kind of template to build the resulting asset name

rspack allows any kind of template, but hash functions are delimited to xxhash64 or md4 (and the hashes are not really from the file content, unlike webpack)

esbuild allows any template, but only supports xxhash64 (postprocessed with base32, which makes it different from the two above)

How do you think we should model this? It certainly seems we should allow users to configure somewhere the file name template to be used to generate the final paths, right? But we should somehow guarantee that the templates are also compatible across bundlers?

In my mind, this wasn't blocking an initial mvp of this feature because users can always update their JS bundler config to use the default asset filename of each bundler, which is what is currently implemented:

esbuild: [name]-[hash][ext]

webpack: [hash][ext]

Wdyt?

Current impl looks good, I tried to rationalise over the idea of coupling, but it's not practical. Thanks for the explanation, it might deserve a blog post.

@davesnx

CHANGES: ## 0.3.1 * Update quickjs dependency to 0.1.2 by @davesnx ## 0.3.0 * browser-ppx: process stritems by @jchavarri in ml-in-barcelona/server-reason-react#127 * Make React.Children.* APIs work as expected by @davesnx in ml-in-barcelona/server-reason-react#130 * Improve global crashes by @davesnx in ml-in-barcelona/server-reason-react#132 * Support assets in `mel.module` by @jchavarri in ml-in-barcelona/server-reason-react#134 * browser_only: don't convert to runtime errors on identifiers or function application by @jchavarri in ml-in-barcelona/server-reason-react#138 * Port `j` quoted strings interpolation from Melange by @jchavarri in ml-in-barcelona/server-reason-react#139 * mel.module: handle asset prefix by @jchavarri in ml-in-barcelona/server-reason-react#140 * Add browser_only transformation to useEffect automatically by @davesnx in ml-in-barcelona/server-reason-react#145 * Append doctype tag on html lowercase by @davesnx in ml-in-barcelona/server-reason-react#136 * Transform Pexp_function with browser_only by @davesnx in ml-in-barcelona/server-reason-react#146 ## 0.2.0 - Remove data-reactroot attr from ReactDOM.renderToString ml-in-barcelona/server-reason-react#129 by @pedrobslisboa - Make useUrl return the provided serverUrl ml-in-barcelona/server-reason-react#125 by @purefunctor - Replace Js.Re implemenation from `pcre` to quickjs b1a3e225cdad1298d705fbbd9618e15b0427ef0f by @davesnx - Remove Belt.Array.push ml-in-barcelona/server-reason-react#122 by @davesnx ## 0.1.0 Initial release of server-reason-react, includes: - Server-side rendering of ReasonReact components (renderToString, renderToStaticMarkup & renderToLwtStream) - `server-reason-react.browser_ppx` for skipping code from the server - `server-reason-react.melange_ppx` for enabling melange bindings and extensions which run on the server - `server-reason-react.belt` a native Belt implementation - `server-reason-react.js` a native Js implementation (unsafe and limited) - `server-reason-react.url` and `server-reason-react.url-native` a universal library with both implementations to work with URLs on the server and the client - `server-reason-react.promise` and `server-reason-react.promise-native` a universal library with both implementations to work with Promises on the server and the client. Based on https://github.com/aantron/promise - `server-reason-react.melange-fetch` a fork of melange-fetch which is a melange library to fetch data on the client via the Fetch API. This fork is to be able to compile it on the server (not running). - `server-reason-react.webapi` a fork of melange-webapi which is a melange library to work with the Web API on the client. This fork is to be able to compile it on the server (not running).

@davesnx

CHANGES: ## 0.3.1 * Update quickjs dependency to 0.1.2 by @davesnx ## 0.3.0 * browser-ppx: process stritems by @jchavarri in ml-in-barcelona/server-reason-react#127 * Make React.Children.* APIs work as expected by @davesnx in ml-in-barcelona/server-reason-react#130 * Improve global crashes by @davesnx in ml-in-barcelona/server-reason-react#132 * Support assets in `mel.module` by @jchavarri in ml-in-barcelona/server-reason-react#134 * browser_only: don't convert to runtime errors on identifiers or function application by @jchavarri in ml-in-barcelona/server-reason-react#138 * Port `j` quoted strings interpolation from Melange by @jchavarri in ml-in-barcelona/server-reason-react#139 * mel.module: handle asset prefix by @jchavarri in ml-in-barcelona/server-reason-react#140 * Add browser_only transformation to useEffect automatically by @davesnx in ml-in-barcelona/server-reason-react#145 * Append doctype tag on html lowercase by @davesnx in ml-in-barcelona/server-reason-react#136 * Transform Pexp_function with browser_only by @davesnx in ml-in-barcelona/server-reason-react#146 ## 0.2.0 - Remove data-reactroot attr from ReactDOM.renderToString ml-in-barcelona/server-reason-react#129 by @pedrobslisboa - Make useUrl return the provided serverUrl ml-in-barcelona/server-reason-react#125 by @purefunctor - Replace Js.Re implemenation from `pcre` to quickjs b1a3e225cdad1298d705fbbd9618e15b0427ef0f by @davesnx - Remove Belt.Array.push ml-in-barcelona/server-reason-react#122 by @davesnx ## 0.1.0 Initial release of server-reason-react, includes: - Server-side rendering of ReasonReact components (renderToString, renderToStaticMarkup & renderToLwtStream) - `server-reason-react.browser_ppx` for skipping code from the server - `server-reason-react.melange_ppx` for enabling melange bindings and extensions which run on the server - `server-reason-react.belt` a native Belt implementation - `server-reason-react.js` a native Js implementation (unsafe and limited) - `server-reason-react.url` and `server-reason-react.url-native` a universal library with both implementations to work with URLs on the server and the client - `server-reason-react.promise` and `server-reason-react.promise-native` a universal library with both implementations to work with Promises on the server and the client. Based on https://github.com/aantron/promise - `server-reason-react.melange-fetch` a fork of melange-fetch which is a melange library to fetch data on the client via the Fetch API. This fork is to be able to compile it on the server (not running). - `server-reason-react.webapi` a fork of melange-webapi which is a melange library to work with the Web API on the client. This fork is to be able to compile it on the server (not running).

jchavarri added 7 commits May 17, 2024 10:58

assets: update mel_module test

8d0691c

mel_module: some basic support for assets

3220a3d

vendor base32 and xxhash

ececa00

use esbuild hash function

9088d12

fmt

89e8e96

Add support for webpack

85613ad

fix ci

390b065

jchavarri requested a review from davesnx May 23, 2024 07:52

jchavarri commented May 23, 2024

View reviewed changes

davesnx approved these changes May 29, 2024

View reviewed changes

davesnx reviewed May 29, 2024

View reviewed changes

jchavarri and others added 4 commits May 30, 2024 06:13

add depexts for xxhash

d69747d

simplify file read

661160b

Remove xxhash from C and written in OCaml (#135)

62d9a3f

remove depexts and unused dune config

b0276bf

davesnx merged commit 40f33e1 into main Jun 19, 2024
2 checks passed

davesnx deleted the mel-module-assets branch June 19, 2024 13:49

jchavarri mentioned this pull request Jun 25, 2024

mel.module: handle asset prefix #140

Merged

davesnx mentioned this pull request Jul 22, 2024

[new release] server-reason-react (0.3.0) ocaml/opam-repository#26269

Merged

davesnx mentioned this pull request Jul 23, 2024

[new release] server-reason-react (0.3.1) ocaml/opam-repository#26275

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Support assets in `mel.module` #134

Support assets in `mel.module` #134

Uh oh!

jchavarri commented May 17, 2024 •

edited

Loading

Uh oh!

jchavarri commented May 23, 2024

Uh oh!

jchavarri May 23, 2024

Uh oh!

davesnx May 29, 2024

Uh oh!

jchavarri May 29, 2024

Uh oh!

davesnx May 29, 2024

Uh oh!

jchavarri May 30, 2024

Uh oh!

davesnx May 29, 2024

Uh oh!

davesnx May 29, 2024

Uh oh!

davesnx May 29, 2024

Uh oh!

jchavarri May 29, 2024

Uh oh!

davesnx May 29, 2024

Uh oh!

jchavarri May 30, 2024 •

edited

Loading

Uh oh!

davesnx May 30, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Support assets in mel.module #134

Support assets in mel.module #134

Uh oh!

Conversation

jchavarri commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jchavarri commented May 23, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jchavarri May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Support assets in `mel.module` #134

Support assets in `mel.module` #134

jchavarri commented May 17, 2024 •

edited

Loading

jchavarri May 30, 2024 •

edited

Loading