ElMassimo / vite_ruby

⚡️ Vite.js in Ruby, bringing joy to your JavaScript experience
https://vite-ruby.netlify.app/
MIT License
1.31k stars 121 forks source link

"The CJS build of Vite's Node API is deprecated." #431

Closed jrochkind closed 3 months ago

jrochkind commented 8 months ago

Using vite-rails 3.0.17, vite_ruby 3.5.0, and vite 5.0.10, I'm getting this deprecation warning output in my console when running assets:precompile.

remote:        Build with Vite complete: /tmp/build_93418466/public/vite
remote:        The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.

I upgraded to vite 5.x after upgrading to vite_ruby 3.5.0, as it seemed to prefer vite 5? But I wonder if it's triggering this deprecation warning in vite 5, and if that can/should be fixed?

jon-codes commented 8 months ago

Enabling "type": "module" in package.json resolves this warning.

jrochkind commented 7 months ago

Sorry if this is a silly question, my JS environment managing knowledge is pretty basic... @jon-codes are you saying that's a change to vite-ruby that is required (vite-ruby's own package.json?), or this is something I just put in my own package.json? Can you provide an example of how to enable "type": "module" in package.json? Thanks!

jon-codes commented 7 months ago

Sure @jrochkind, you can enable it in your own package.json.

The "type" key is a Node.js-specific option, which you can find documented here. Using the "module" value will cause Node.js to interpret Javascript files as ECMAScript modules (ESM) rather than CommonJS modules (CJS).

Here's an example from my vite-ruby project:

// package.json

{
  "type": "module",
  "scripts": {
    "format": "prettier --check .",
    "format:fix": "prettier --write .",
    "lint": "eslint .",
    "lint:fix": "eslint --fix ."
  },
  "dependencies": {
    "@hotwired/turbo-rails": "^8.0.2"
  },
  "devDependencies": {
    "@fullhuman/postcss-purgecss": "^5.0.0",
    "@types/node": "^20.11.19",
    "autoprefixer": "^10.4.17",
    "eslint": "^8.56.0",
    "eslint-plugin-simple-import-sort": "^12.0.0",
    "postcss": "^8.4.35",
    "postcss-nesting": "^12.0.2",
    "prettier": "^3.2.5",
    "vite": "^5.1.3",
    "vite-plugin-rails": "^0.5.0"
  },
  "engines": {
    "node": "20.11.1",
    "npm": ">=10"
  },
  "os": [
    "linux",
    "darwin"
  ]
}

I'm using a .js file extension for the Vite config. If using .ts the compiler may create CJS modules by default rather than ESM modules.

// vite.config.js

/* esbuild-env node */
import { dirname, resolve } from "node:path";
import { fileURLToPath } from "node:url";

import { defineConfig } from "vite";
import RailsPlugin from "vite-plugin-rails";

// ESM modules do not have `__filename` and `__dirname` constants, unlike CJS modules.
// See: https://nodejs.org/api/esm.html#differences-between-es-modules-and-commonjs 
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);

export default defineConfig({
  plugins: [RailsPlugin()],
  resolve: {
    alias: {
      "@vendor": resolve(__dirname, "vendor"),
      "@fonts": resolve(__dirname, "assets/fonts"),
      "@icons": resolve(__dirname, "assets/icons"),
      "@images": resolve(__dirname, "assets/images"),
      "@stylesheets": resolve(__dirname, "assets/stylesheets"),
    },
  },
});
jrochkind commented 7 months ago

Thank you for that explanation! I will have to go through it more slowly and try things out.

I'm using a .js file extension for the Vite config. If using .ts the compiler may create CJS modules by default rather than ESM modules.

vite-ruby generator right now creates vite.config.ts

https://github.com/ElMassimo/vite_ruby/blob/369facf440f41162efee825a87d9491ff83a03b8/vite_ruby/lib/vite_ruby/cli/install.rb#L73

Do you think this needs/should be changed? I think the file does not actually use anything that's non-JS typescript or anything like that? Not sure why it has had .ts from the start. @ElMassimo ?

It looks like possibly the generated file needs to be different too, per your comments on __filename and __dirname? I am not a maintainer, but I wonder if you want to submit a PR, as you understand what's up?

loftwah commented 3 months ago

To resolve this issue on my system I replaced my vite.config.ts with vite.config.mjs. I wrote how I did it here. Would this be a suitable solution going forward? It removes the warning.

Here is my vite.config.mjs

import { defineConfig } from 'vite';
import RubyPlugin from 'vite-plugin-ruby';
import tailwindcss from 'tailwindcss';
import autoprefixer from 'autoprefixer';

export default defineConfig({
  plugins: [
    RubyPlugin(),
  ],
  css: {
    postcss: {
      plugins: [
        tailwindcss,
        autoprefixer,
      ],
    },
  },
  assetsInclude: ['**/*.png', '**/*.jpg', '**/*.jpeg', '**/*.gif', '**/*.svg'],
});

EDIT: It turns out the first reply is all I needed. Adding "type": "module", to my package.json resolved it.

jrochkind commented 3 months ago

Thanks @loftwah

It looks to me like simply renaming my existing vite.config.ts to vite.config.mjs has resulted in the deprecation message showing up.

I didn't need to make any changes to vite.config.[ts|mjs], nor did I need to make any changes to package.json -- which does not contain a type: "module" key.

Just name change of vite.config from .ts to .mjs.

@ElMassimo Does this make sense to you? Would it make sense to change the vite-ruby generator to generate filename vite.config.mjs instead of .ts (assuming it includes no typescript, which it doens't appear to?)

We could prepare a PR, which changes the file, references to it, examples, etc. eg https://github.com/ElMassimo/vite_ruby/blob/93ae26fc99942403a785d0751abffb184c5bc579/vite_ruby/lib/vite_ruby/cli/install.rb#L73

jrochkind commented 3 months ago

Ah or .mts if you want typescript? (But why do you want typescript?)

I noticed https://github.com/ElMassimo/vite_ruby/issues/431 too

@ElMassimo some maintainer advice would be appreciated, would love to get a PR in to avoid this confusion for newcomers!

mattbrictson commented 3 months ago

Ultimately I believe the solution, as others have mentioned, is to set "type": "module" in your app's package.json. However, this seems to be a manual step that the vite_ruby installer doesn't handle automatically, which is a bad experience for new users.

This is the package.json generated by the vite_ruby installer, in its entirety:

{
  "devDependencies": {
    "vite": "^5.3.4",
    "vite-plugin-ruby": "^5.0.0"
  }
}

Perhaps one step in the right direction would be for the installer to initialize the package.json with the right type?

I.e. change this line:

https://github.com/ElMassimo/vite_ruby/blob/93ae26fc99942403a785d0751abffb184c5bc579/vite_ruby/lib/vite_ruby/cli/install.rb#L82

to this:

unless package_json.exist?
  write(package.json, <<~JSON)
    {
      "private": true,
      "type": "module"
    }
  JSON
end
jrochkind commented 3 months ago

I think it's important the out of the box generators for vite-ruby/rails genreate something that does not produce deprecation warnings!

I'm not a JS expert... just changing the suffix of the vite.config to .mts seemed to me safer than editing the package.json, but I don't fully understand the implications and defer to you!

In fact, not understanding the implications even for myself is part of why I want someone who understands this better to have the vite-ruby generators do the right thing, so I can just do what they do!

mattbrictson commented 3 months ago

@jrochkind the way I see it, ESM (meaning ES modules, which is what "type": "module" enables by default) is the future, so for a brand new project with no existing package.json, I think it is best to initialize it with "type": "module". If you were to use Vite directly, outside of Rails, that's what the official Vite app templates recommend. E.g. both the "vanilla" and React Vite templates start you off with "type": "module":

However, I see your point that for an existing project, changing the package.json type could have unforeseen consequences, because it changes the default of how all .js and .ts files in the project are interpreted. In that case, it's probably safer for the vite_ruby installer to leave the package.json type alone and simply use the .mts extension for vite.config.mts.

mattbrictson commented 3 months ago

@jrochkind sorry, to clarify: I am adding some ideas to this thread for possible fixes, just to further the conversation, and for maintainers to see when reviewing this issue. I do not have decision-making authority on this project. 🙂

jon-codes commented 3 months ago

@jrochkind @mattbrictson I was about to reply with a similar observation about ESM being the preferred/default module type for Vite projects in Node. Other observations:

I think the major downside, as mentioned, is that the change is riskier & requires updating more files than just updating the extension to .mts. I started working on a branch to implement "type": "module", which I think is relatively complete, and it touches 9 files due to documentation & tests.

jrochkind commented 3 months ago

Thank you that's all very helpful! I will add type: module to my own project, now that I know it's the preferred module type for vite projects!

I have no opinion of my own of what is best for vite-ruby generator, beyond thinking that vite-ruby generator ought to genreate something that does not cause deprecation warnings! But unless maintainer shows up to welcome a PR... ?

jon-codes commented 3 months ago

I'll clean up my branch using the "type": "module" approach and submit a PR shortly

ElMassimo commented 3 months ago

Perhaps one step in the right direction would be for the installer to initialize the package.json with the right type?

Agreed, happy to take a PR with that change, as it's safe for new projects.

Given the deprecation of the CJS API, it would be viable to detect an existing package.json, and rename vite.config.ts to vite.config.mts during installation if it's not "type": "module".

Finally, we should add vite.config.mts to the default watched files now that this extension might become more common.

ElMassimo commented 3 months ago

Thanks for the discussion folks, and @mattbrictson for suggesting ESM by default for new installations.

This is now released in vite_ruby-3.6.1, no need to upgrade though.

Users facing this issue can refer to the Troubleshooting guide.

mattbrictson commented 3 months ago

@ElMassimo I'm getting an error running vite install (I think due to a typo in the code I suggested):

vite_ruby-3.6.1/lib/vite_ruby/cli/install.rb:84:in `install_js_dependencies': undefined local variable or method `package' for an instance of ViteRuby::CLI::Install (NameError)

      write(package.json, <<~JSON)

It should be

write(package_json, <<~JSON)

_Edit: package_json, not package.json or "package.json"._

ElMassimo commented 3 months ago

Good catch, it looks like there's no coverage for this path. I'll cut a new release.