lucky icon indicating copy to clipboard operation
lucky copied to clipboard

Evaluate future of mix integration

Open jwoertink opened this issue 3 years ago • 11 comments

We originally integrated laravel-mix as default because when Lucky was first being built in 2017, webpack was the top choice, and mix provided really great configurations to wrap webpack.

Now in 2022, Laravel has deprecated mix https://laravel.com/docs/9.x/mix in favor of using Vite https://vitejs.dev/ This seems like a pretty decent choice seeing that it's using esbuild under the hood, and comes wrapped up with a lot of nice tooling like rollup and such. However, on my initial test of this, I was getting javascript compiler crashes, and found some of the integration docs to be very confusing.

I got this error with almost no javascript in the app. I let it sit for about ~5min and then I would get this.

// assets       | <--- JS stacktrace --->
// assets       |
// assets       | FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
// assets       |  1: 0xafedf0 node::Abort() [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  2: 0xa1814d node::FatalError(char const*, char const*) [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  3: 0xce795e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  4: 0xce7cd7 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  5: 0xeb16b5  [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  6: 0xeb21a4  [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  7: 0xec0617 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  8: 0xec39cc v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       |  9: 0xe862ec v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationType, v8::internal::AllocationOrigin) [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       | 10: 0x11f3156 v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long*, v8::internal::Isolate*) [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       | 11: 0x15c9ed9  [/home/jeremy/.asdf/installs/nodejs/16.6.1/bin/node]
// assets       | Aborted (core dumped)
// assets       | error Command failed with exit code 134.
// assets       | info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
// assets       | error Command failed with exit code 134.
// assets       | info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
// assets       | Done

The other issue I ran in to was, after changing a JS file, vite would rebuild the assets, but never notify Lucky that this was changed.

We don't necessarily have to switch to Vite if there's a better solution, but no javascript also isn't really an option. Here's the items that a new solution must meet:

  • Typesafe assets - If you specify an image, script, style, or any sort of asset file, the compiler should fail with a nice error telling you which asset you called, and where it tried to find it. (e.g. you called "logo.png", but it wasn't found in /assets/images/logo.png)
  • Easy escape hatch - We should have some default setup that allows you to get to production quick with a custom front-end, but you should also be able to swap that out for something else if you need without much trouble.
  • Minimal setup - It sucks that node is required when building sites, but that's just the nature of doing business these days. Whatever solution we choose shouldn't require a bunch of additional setup (i.e. must also install Go, or Rust, or have additional software installed, etc...)
  • Long lived support - I don't want to go through this every other year, so whatever is chosen should at least look promising enough to be around for the next ~4 years. I'm ok doing a reevaluation 4-5 years later, but changing now, then next year having to do it again would be a pain.

jwoertink avatar Aug 22 '22 17:08 jwoertink

Just for adding in Vite support, here's what I did:

First, hack these two lines to match how Vite generates a manifest.json file differently from mix https://github.com/luckyframework/lucky/blob/393b9e937afe9bee48b4a301f44d160a566afa48/src/run_macros/generate_asset_helpers.cr#L41-L42

manifest.as_h.each do |key, value|
  key = key[4..]
  puts %({% ::Lucky::AssetHelpers::ASSET_MANIFEST["#{key}"] = "#{value["file"].as_s}" %})
end

Updated the name of my manifest Lucky looks for:

# src/app.cr
Lucky::AssetHelpers.load_manifest "public/manifest.json"

Added Vite to my package.json

{
  "license": "UNLICENSED",
  "private": true,
  "dependencies": {
    "@rails/ujs": "^6.0.0",
    "modern-normalize": "^1.1.0",
    "turbolinks": "^5.2.0"
  },
  "scripts": {
    "dev": "yarn run vite",
    "build": "yarn run vite build",
    "preview": "yarn run vite preview",
    "watch": "yarn run vite build --watch"
  },
  "devDependencies": {
    "vite": "^3.0.8"
  }
}

Added a vite.config.js file

import { defineConfig } from 'vite';

export default defineConfig({
  server: {
    watch: {
      include: ['**/src/js/**', '**/src/css/**'],
    },
  },
  build: {
    outDir: './public',
    emptyOutDir: false,
    manifest: true,
    rollupOptions: {
      input: './src/js/app.js'
    }
  }
});

Updated app.js with some stuff

import styles from "../css/app.css";

import logo from "../images/lucky_logo.png";

import Rails from "@rails/ujs";

That's all it took, but I can't guarantee if that was the best way. I saw in their docs mentioning the script tag should say type="module", but I didn't seem to need to do that...

jwoertink avatar Aug 22 '22 17:08 jwoertink

Looks like Vite has an open issue about memory https://github.com/vitejs/vite/issues/2433 issues.

jwoertink avatar Aug 23 '22 18:08 jwoertink

+1 for moving away from Mix.

I moved to ESBuild, which has worked great and is super fast. Here's the commit: https://github.com/stephendolan/lucky_jumpstart/pull/567

stephendolan avatar Aug 23 '22 22:08 stephendolan

Rails' new approach also seems cool! I don't understand it, but I like the simplicity: https://github.com/rails/importmap-rails

stephendolan avatar Aug 24 '22 13:08 stephendolan

One other option I wanted to drop for discussion is Parcel! https://parceljs.org/

stephendolan avatar Aug 24 '22 13:08 stephendolan

Import maps is still not a standard and supported natively only in Chrome. Support in Firefox is behind a flag and seems like it won't be enabled until import maps became a standard.

vlazar avatar Aug 24 '22 16:08 vlazar

Import maps is still not a standard and supported natively only in Chrome. Support in Firefox is behind a flag and seems like it won't be enabled until import maps became a standard.

Yeah, there's a shim here to get it working for all the evergreens! https://github.com/guybedford/es-module-shims

stephendolan avatar Aug 24 '22 16:08 stephendolan

@stephendolan I'm aware, but it's still betting on a non standard feature.

vlazar avatar Aug 24 '22 16:08 vlazar

@stephendolan I'm aware, but it's still betting on a non standard feature.

Yeah, I don't use it, but it's worth considering as a part of the discussion, IMO!

stephendolan avatar Aug 24 '22 16:08 stephendolan

fyi, importmaps are now more widely available: https://caniuse.com/import-maps

Also, I plan to contribute to lucky_cli to add a more minimal solution for Lucky apps, that I'm currently building on a hobby project.

mdwagner avatar Jan 20 '23 02:01 mdwagner

I don't know much about it, but there is another interesting bundler, written in Rust: https://turbo.build/ Unfortunately, I don't know much about it as I only used Vite.

notramo avatar May 08 '23 14:05 notramo