luckyframework / lucky

A full-featured Crystal web framework that catches bugs for you, runs incredibly fast, and helps you write code that lasts.
https://luckyframework.org
MIT License
2.57k stars 156 forks source link

Evaluate future of mix integration #1724

Open jwoertink opened 1 year ago

jwoertink commented 1 year ago

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:

jwoertink commented 1 year ago

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 commented 1 year ago

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

stephendolan commented 1 year ago

+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 commented 1 year ago

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

stephendolan commented 1 year ago

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

vlazar commented 1 year ago

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.

stephendolan commented 1 year ago

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

vlazar commented 1 year ago

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

stephendolan commented 1 year ago

@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!

mdwagner commented 1 year ago

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.

notramo commented 1 year ago

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.