-
Notifications
You must be signed in to change notification settings - Fork 16
Add Miso #43
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
base: master
Are you sure you want to change the base?
Add Miso #43
Conversation
20eef8e
to
93f9e8f
Compare
-path './Setup.hs' -prune -o \ | ||
-name '*.hs' \ | ||
-print \ | ||
>> "$HIE_BIOS_OUTPUT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All that's changed here from the upstream version are the excluded paths in the call to find
. Would it be better to modify that script to accept ignore directories as arguments instead? Then we could avoid the repetition and partially revert the changes to hie.yaml
.
Ignoring miso
allows us to load that directory as a plain Cabal project, as seen in hie.yaml
.
Ignoring dist-newstyle
means we don't confuse HLS by trying to load dependencies of the frontend in to the backend session. For some reason, this manifests as some files being unaware of type class instances from others, as seen here. While other users in that thread are presumably not all using Haskell for the frontend, I suspect the root cause may be the same. For example, they might have called an unwrapped Cabal directly, thus creating dist-newstyle
. So this might be worth upstreaming too.
Ignoring Setup.hs
isn't strictly necessary, but it can't hurt. Incidentally, is there any good reason why there's one here at all? I'm pretty sure that having a default setup script has been redundant for longer than IHP has existed.
<script src={assetPath "/helpers.js"}></script> | ||
<script src={assetPath "/ihp-auto-refresh.js"}></script> | ||
<script src={assetPath "/app.js"}></script> | ||
+ <script src={assetPath "/miso/index.js"} type="module"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These patch files of course mean that this repo isn't actually runnable. I developed these changes in a repo created by ihp-new
and then copied them here. If we make further changes here, is there a good way to test them, other than building a modified version of ihp-new
which can point to this branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently there is no better way to test this, the boilerplate is not changed that often typically
"$$hs_wasm_libdir"/post-link.mjs --input "$$hs_wasm_path" --output static/miso/ghc_wasm_jsffi.js | ||
env -i GHCRTS=-H64m $(shell type -P wizer) --allow-wasi --wasm-bulk-memory true --inherit-env true --init-func _initialize -o static/miso/bin.wasm "$$hs_wasm_path" | ||
wasm-opt -Oz static/miso/bin.wasm -o static/miso/bin.wasm | ||
wasm-tools strip -o static/miso/bin.wasm static/miso/bin.wasm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure how this Makefile
is supposed to work, as I haven't studied the included ${IHP}/Makefile.dist
in much depth. For one thing, since we have JS_FILES
but no WASM_FILES
, I've made bin.wasm
a side effect of producing one of the JS files, which is a bit of a hack.
Also, there are no dependencies set for the new JS targets, so we end up requiring -B
to force a rebuild. There aren't any set for the Elm branch either, so maybe this is somehow fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we switched to nix flakes, my new preferred way of doing these frontend operations is as a flake package. This way the frontend and backend can be compiled in parallel (very useful once a project hits a certain size. Also e.g. a commit changing only miso files will then not trigger a full backend rebuild).
Here is a good starting point (I copied this from one of our projects and tweaked it a bit, but it's not fully working yet):
{
inputs = {
};
outputs = inputs@{ self, nixpkgs, ihp, flake-parts, systems, ... }:
flake-parts.lib.mkFlake { inherit inputs; } {
systems = import systems;
imports = [ ihp.flakeModules.default ];
perSystem = { pkgs, system, config, ... }: {
ihp = {};
devenv.shells.default = {
# Custom processes that don't appear in https://devenv.sh/reference/options/
processes.frontend.exec = ''
'';
packages.frontend =
let
filter = ihp.inputs.nix-filter.lib;
in pkgs.stdenv.mkDerivation {
name = "${config.ihp.appName}-frontend";
src = filter {
root = ./Frontend;
include = ["src" "types" (filter.matchExt "js") (filter.matchExt "ts") (filter.matchExt "tsx") (filter.matchExt "json") (filter.matchExt "css")];
exclude = ["node_modules"];
};
nativeBuildInputs = [inputs.ghc-wasm-meta.packages.${system}.all_9_10];
allowedReferences = [];
buildPhase = ''
wasm32-wasi-cabal build Frontend
hs_wasm_path=$(shell wasm32-wasi-cabal list-bin Frontend)
hs_wasm_libdir=$(shell wasm32-wasi-ghc --print-libdir)
"$$hs_wasm_libdir"/post-link.mjs --input "$$hs_wasm_path" --output static/miso/ghc_wasm_jsffi.js
env -i GHCRTS=-H64m $(shell type -P wizer) --allow-wasi --wasm-bulk-memory true --inherit-env true --init-func _initialize -o static/miso/bin.wasm "$$hs_wasm_path"
wasm-opt -Oz static/miso/bin.wasm -o static/miso/bin.wasm
wasm-tools strip -o static/miso/bin.wasm static/miso/bin.wasm
'';
};
packages.optimized-prod-server-with-frontend = self.packages."${system}".optimized-prod-server.overrideAttrs (finalAttrs: previousAttrs: {
name = "${previousAttrs.name}-with-frontend";
nativeBuildInputs = previousAttrs.nativeBuildInputs ++ [ self.packages."${system}".frontend ];
preBuild = ''
mkdir -p static/Frontend
ln -s ${self.packages."${system}".frontend}/main.js static/Frontend/main.js
ln -s ${self.packages."${system}".frontend}/main.css static/Frontend/main.css
'';
});
};
};
}
Then nix build .#frontend
should build the miso frontend. And nix build .#optimized-prod-server-with-frontend
will e.g. provide a production package with the miso frontend copied into it.
(Independent of this we likely should remove the whole Makefile process in favour of nix in the next major IHP version)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm certainly keen on the idea of dropping the Makefiles! I might even wait until that release and see what changes you make for the other frontends before proceeding with this. Especially as this is already somewhat blocked by other non-IHP things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of waiting :D We'll likely only drop full Makefile support in the next major version. It's unclear when that will be released. I'd prefer that we have miso support even with the current version. Better to iterate on this instead of waiting for the perfect design IMO 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay! If you're happy releasing the less-than-perfect version then so am I. I'll do what I can to get this over the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do have some thoughts on frontend+backend code sharing, i.e. actually taking advantage of both being written in the same language, but I'll wait until this is merged before writing that up. I expect it might take some serious project refactoring, and I'm not familiar enough with IHP's internals to have much idea what that would look like. This PR pretty much isolates the two and treats them totally separately, aside from using the same haskell-language-server
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks 👍 happy to read your thoughts then
@@ -0,0 +1,12 @@ | |||
optional-packages: miso |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have expected to be able to use optional-packages: . miso
here, but for some reason the frontend build then still complains about ihp
being an unknown library.
projectPath = ./.; | ||
packages = with pkgs; [ | ||
# Native dependencies, e.g. imagemagick | ||
inputs.ghc-wasm-meta.packages.${system}.all_9_10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IHP is currently pinned to GHC 9.8, which is not an option for the frontend, since there's too much Wasm functionality missing. In fact I believe development of the Wasm backend is mostly targeting 9.12 at this point, with patches being selectively backported to 9.10.
Currently we do get away with using a native GHC 9.8, including for HLS for the frontend, alongside a Wasm GHC 9.10, but this isn't ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we can upgrade IHP to GHC 9.10 soon
processes = { | ||
# Uncomment if you use tailwindcss. | ||
# tailwind.exec = "tailwindcss -c tailwind/tailwind.config.js -i ./tailwind/app.css -o static/app.css --watch=always"; | ||
frontend.exec = "./watch-frontend"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gives us a form of reload-on-save, though it's a little slow, since we recompile a new bin.wasm
. We essentially run the same commands in watch-frontend
as in the Makefile
, just without optimisations.
Another approach would be jsaddle-warp
, used similarly to here, which enables much faster reloads by using a native GHCI and sending commands to the browser telling it what JS to execute. However, it's unclear how well this would work with IHP and, besides, jsaddle-warp
is known to have major issues and doesn't work at all in some browsers, e.g. Firefox. In the long run, the jsaddle-warp
workflow is expected to be replaced with better GHCI support for Wasm, allowing running in the browser with the bytecode interpreter. So the approach here might suffice until then.
Note that the webpage reloads when saving a source file from the frontend component, which is nice, although it tends to reload before the watch script has finished compiling bin.wasm
, so in practice saving the file again a few seconds later is necessary in order to see the new result. I guess there's some existing process as part of IHP which is watching all Haskell files, and should ultimately be told to treat frontend source files differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The watch-frontend
script will need changes for MacOS compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the webpage reloads when saving a source file from the frontend component, which is nice, although it tends to reload before the watch script has finished compiling
bin.wasm
, so in practice saving the file again a few seconds later is necessary in order to see the new result. I guess there's some existing process as part of IHP which is watching all Haskell files, and should ultimately be told to treat frontend source files differently.
@mpscholten This is the one part of this workflow that's seriously janky and could do with being fixed before this is released. Could I get some pointers on how the current file watching works and whether it might be able to be adjusted to delay the page refresh until after we have a new bin.wasm
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes happy to help.
Most of the file watching happens in IHP.IDE.FileWatcher
. The file watcher groups the changes into .hs
file changes, .sql
file changes and .css
file changes. Other file events are ignored. So the bin.wasm
is not directly causing this. Instead the file change to the miso haskell files is triggering the refresh.
This will eventually lead to this function: https://github.com/digitallyinduced/ihp/blob/master/ihp-ide/exe/IHP/IDE/DevServer.hs#L167 causing :r
inside the ghci
process that is running the app.
One idea: we could ignore the miso directory from the file watcher. This way nothing will happen when miso is recompiled. Then we could change the miso compile script to touch
the static/miso/index.js
to cause a reload of the JS files (which would cause a reload of the miso app).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing we should do to speed this up is use Miso's support for isomorphic pre-rendering. In fact, we should do this for production as well.
Anyway, with proper Wasm GHCI support now looking like it's only a few weeks away, I'm likely to wait for that rather than investing time in an approach which will soon be obsolete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good 👍 let's wait then
doc <- MaybeT currentDocument | ||
parent <- MaybeT $ getElementById doc ("miso" :: MisoString) | ||
child <- MaybeT $ getFirstElementChild parent | ||
pure (parent, child) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This replicates the functionality of the Elm example: a "not working" placeholder text is initially present, and removed when the Elm/Miso runtime has finished setting up. Things are complicated by the fact that, unlike Elm, Miso inserts itself as a child of the given node, rather than taking it over. So we have to manually call some DOM functions to remove the placeholder.
I've made this a separate commit seeing as the increased complexity in terms of dependencies and imports is such that it might honestly not be worth it, so we may wish to revert.
Alternatively, we could look in to the possibility of adapting Miso to have the option of replicating the Elm behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The need for a :: MisoString
annotation is a common problem when using jsaddle-dom
, which tends to accept arbitrary string-like types, and thus doesn't play well with OverloadedStrings
in its current form.
GHC 9.12's NamedDefaults
would help here.
// Initializes Miso on Turbolinks transition | ||
document.addEventListener("turbolinks:load", (e) => { | ||
initialize(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is essentially a fusion of the JS wrappers from IHP's Elm boilerplate branch and Tweag's Miso Wasm examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, great work 👍 just took a first look
<script src={assetPath "/helpers.js"}></script> | ||
<script src={assetPath "/ihp-auto-refresh.js"}></script> | ||
<script src={assetPath "/app.js"}></script> | ||
+ <script src={assetPath "/miso/index.js"} type="module"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently there is no better way to test this, the boilerplate is not changed that often typically
"$$hs_wasm_libdir"/post-link.mjs --input "$$hs_wasm_path" --output static/miso/ghc_wasm_jsffi.js | ||
env -i GHCRTS=-H64m $(shell type -P wizer) --allow-wasi --wasm-bulk-memory true --inherit-env true --init-func _initialize -o static/miso/bin.wasm "$$hs_wasm_path" | ||
wasm-opt -Oz static/miso/bin.wasm -o static/miso/bin.wasm | ||
wasm-tools strip -o static/miso/bin.wasm static/miso/bin.wasm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we switched to nix flakes, my new preferred way of doing these frontend operations is as a flake package. This way the frontend and backend can be compiled in parallel (very useful once a project hits a certain size. Also e.g. a commit changing only miso files will then not trigger a full backend rebuild).
Here is a good starting point (I copied this from one of our projects and tweaked it a bit, but it's not fully working yet):
{
inputs = {
};
outputs = inputs@{ self, nixpkgs, ihp, flake-parts, systems, ... }:
flake-parts.lib.mkFlake { inherit inputs; } {
systems = import systems;
imports = [ ihp.flakeModules.default ];
perSystem = { pkgs, system, config, ... }: {
ihp = {};
devenv.shells.default = {
# Custom processes that don't appear in https://devenv.sh/reference/options/
processes.frontend.exec = ''
'';
packages.frontend =
let
filter = ihp.inputs.nix-filter.lib;
in pkgs.stdenv.mkDerivation {
name = "${config.ihp.appName}-frontend";
src = filter {
root = ./Frontend;
include = ["src" "types" (filter.matchExt "js") (filter.matchExt "ts") (filter.matchExt "tsx") (filter.matchExt "json") (filter.matchExt "css")];
exclude = ["node_modules"];
};
nativeBuildInputs = [inputs.ghc-wasm-meta.packages.${system}.all_9_10];
allowedReferences = [];
buildPhase = ''
wasm32-wasi-cabal build Frontend
hs_wasm_path=$(shell wasm32-wasi-cabal list-bin Frontend)
hs_wasm_libdir=$(shell wasm32-wasi-ghc --print-libdir)
"$$hs_wasm_libdir"/post-link.mjs --input "$$hs_wasm_path" --output static/miso/ghc_wasm_jsffi.js
env -i GHCRTS=-H64m $(shell type -P wizer) --allow-wasi --wasm-bulk-memory true --inherit-env true --init-func _initialize -o static/miso/bin.wasm "$$hs_wasm_path"
wasm-opt -Oz static/miso/bin.wasm -o static/miso/bin.wasm
wasm-tools strip -o static/miso/bin.wasm static/miso/bin.wasm
'';
};
packages.optimized-prod-server-with-frontend = self.packages."${system}".optimized-prod-server.overrideAttrs (finalAttrs: previousAttrs: {
name = "${previousAttrs.name}-with-frontend";
nativeBuildInputs = previousAttrs.nativeBuildInputs ++ [ self.packages."${system}".frontend ];
preBuild = ''
mkdir -p static/Frontend
ln -s ${self.packages."${system}".frontend}/main.js static/Frontend/main.js
ln -s ${self.packages."${system}".frontend}/main.css static/Frontend/main.css
'';
});
};
};
}
Then nix build .#frontend
should build the miso frontend. And nix build .#optimized-prod-server-with-frontend
will e.g. provide a production package with the miso frontend copied into it.
(Independent of this we likely should remove the whole Makefile process in favour of nix in the next major IHP version)
projectPath = ./.; | ||
packages = with pkgs; [ | ||
# Native dependencies, e.g. imagemagick | ||
inputs.ghc-wasm-meta.packages.${system}.all_9_10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we can upgrade IHP to GHC 9.10 soon
This adds a Miso frontend example, similar to the Elm one. Note that we don't actually want to merge this to
master
, sinceihp-new
expects examples to live on their own branches, but I don't think GitHub has a way of saying "I'd like you to create a new branch based on mine".