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

Fix assets windows #31

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

kyleplump
Copy link

this is a fix related to the following conversations:

TL;DR: hanami dev and asset compilation doesnt work on windows
(the issue where the conversation (mostly) took place is hanami/hanami#1463)

this is part one of likely a few PR's


assets.json was not being created on windows platforms, nor were assets being appropriately built / hashed
this PR includes 2 fixes which gets the process working on windows:

  1. we are using the fs-extra package's writeJSON method to write json files (the manifests). this does not create directories if they do not already exist. for some reason this works on *nix systems, but not windows. the outputJSON function will create a directory (by default public/assets isnt created during hanami new).
    Error when running on windows jprichardson/node-jsonfile#60

  2. file path references were broken all over the place, because turns out glob.sync doesnt take anything other than posix format. i introduced a small helper function to convert all paths to use the appropriate path separator

hanami assets compile now appropriately creates all asset files correctly (although hanami dev and hanami assets watch still aren't working -> thats a separate issue and a separate incoming PR on a diff repo :) )

please lmk what you think!

@pcopissa
Copy link

pcopissa commented Nov 12, 2024

I tried to apply the fix to my copy of esbuild.js and esbuild-plugin.js and rerun bundle exec hanami assets compile. It now displays this:

[test]
[test]   public\assets\_.._\_.._\app\assets\js\app-LYGD2ZET.css      107b
[test]   public\assets\_.._\_.._\app\assets\js\app-HHOJ7CRY.js        53b
[test]   public\assets\_.._\_.._\app\assets\js\app-LYGD2ZET.css.map  274b
[test]   public\assets\_.._\_.._\app\assets\js\app-HHOJ7CRY.js.map    93b
[test]
[test] Done in 15ms

I now have a non-empty public/assets/assets.json.
I also have public/assets/favicon-A1725F7F.ico and a strangely named folder public/assets/_.._/_.._/app/assets/js with 4 files:

app-HHOJ7CRY.js
app-HHOJ7CRY.js.map
app-LYGD2ZET.css
app-LYGD2ZET.css.map

FWIW: On Linux I have 6 files in public/assets :

app-GVDAEYEC.css
app-LSLFPUMX.js
assets.json
app-GVDAEYEC.css.map
app-LSLFPUMX.js.map
favicon-B8DAB077.ico

For completeness here is the message output by bundle exec hanami assets compile on Linux:

[test211]
[test211]   public/assets/app-GVDAEYEC.css      107b
[test211]   public/assets/app-LSLFPUMX.js        53b
[test211]   public/assets/app-GVDAEYEC.css.map  249b
[test211]   public/assets/app-LSLFPUMX.js.map    93b
[test211]
[test211] ⚡ Done in 87ms

Also, on Windows public/assets/assets.json looks like this:

{
  "images/favicon.ico": {
    "url": "/assets/favicon-A1725F7F.ico"
  },
  "app.js": {
    "url": "/assets/_.._/_.._/app/assets/js/app-HHOJ7CRY.js"
  },
  "app.css": {
    "url": "/assets/_.._/_.._/app/assets/js/app-LYGD2ZET.css"
  }
}

Nonetheless, I still have a no asset found for "favicon.ico" error when requesting my /u/v page so the asset compilation is not the whole story.
Since I need to move on, I simply commented the <%= favicon_tag %> in the app/templates/layouts/app.html.erb and that got me the /u/v page properly served.
Out of curiosity I deleted app/assets/images/favicon.ico, and re-run bundle hanami assets compile. Indeed the images/favico.ico line has now disappeared from public/assets/assets.json.

TL;DR:

  • proposed PR does not fix the no asset found for "favicon.ico" issue
  • proposed PR causes hanami assets compileto create assets files in different directory than on Linux but CSS and JS assets are properly served.
  • proposed PR causes hanami assets compile to create assets.json in the expected directory public/assets.

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