squint-cljs / vite-plugin-squint

A plugin to compile .cljs source code with squint using vite
1 stars 3 forks source link

Tailwind not recompiling on changes to `.cljs` files #19

Open rmrt1n opened 3 months ago

rmrt1n commented 3 months ago

Tailwind recompiles classes on changes to .js files, but not for .cljs files. Any ideas on why this happens?

martinklepsch commented 3 months ago

Is this the repro case @rmrt1n?

https://github.com/rmrt1n/squint-solid-demo

Original Slack conversation: https://clojurians.slack.com/archives/C03U8L2NXNC/p1712907189474459

rmrt1n commented 3 months ago

Yes, I forgot to include it in the issue desc.

martinklepsch commented 3 months ago

Just looked at this briefly and noticed that it seems to pick up on new classes correctly when running vite build --watch. Not sure why that is but I guess it means this is at least in part related to hot module reloading etc.

cc @brandonstubbs

brandonstubbs commented 2 months ago

I spent some time that day digging into it, but needed some more time to really go down the rabbit hole.

Below are my findings:

Tailwind changes are not being picked up as vite's internal postcss plugin is only checking changes for physical files. The module returns a resolveId of .cljs.jsx that would trick vite into thinking that there was a physical .cljs.jsx on the filesystem but it doesn't exist. This was the approach that worked the best when integrating into the other plugins (react/solid/qwik).

Virtual Modules

I tried converting the plugin to emit virtual modules for the .cljs.jsx files. You can see the code below:

import { compileString } from "squint-cljs";
import path, { dirname } from "path";
import fs from "fs";

export default function viteSquint(opts = {}) {
  const squint = {
    name: "squint_compile",
    enforce: "pre",
    async load(id) {
      // load virtual files
      if (/^\0.*.cljs.jsx$/.test(id)) {
        // TODO: macros
        // TODO: squint source mapping
        const file = id.substring(1).replace(/.jsx$/, "");
        const code = await fs.promises.readFile(file, "utf-8");
        const compiled = compileString(code);
        return { code: compiled, map: null };
      }
    },
    resolveId(id, importer, options) {
      // importing cljs file
      if (!/\0.*/.test(id) && /\.cljs$/.test(id)) {
        // make path relative if absolute
        if (id.startsWith("/")) {
          id = id.substring(1);
        }
        // if it's being imported from a virtual module, remove the \0
        if (/^\0.*.cljs.jsx$/.test(importer)) {
          importer = importer.substring(1);
        }
        const absolutePath = path.resolve(dirname(importer), id);
        // return the virtual module id
        return "\0" + absolutePath + ".jsx";
      }

      // importing another file from a module, vite doesn't resolve paths from
      // virtual modules, so we need to resolve it ourselves
      if (/^\0.*.cljs.jsx$/.test(importer)) {
        const realPath = dirname(importer.substring(1));
        const absolutePath = path.resolve(realPath, id);
        // we only return if the physical file exists, otherwise it's a library import
        if (fs.existsSync(absolutePath)) {
          return absolutePath;
        }
      }

    },
    handleHotUpdate({file, server, modules }) {
      if (/\.cljs$/.test(file)) {
        // this needs to be the same id returned by resolveId this is what
        // vite uses as the modules identifier
        const resolveId = "\0" + file + ".jsx";
        const module = server.moduleGraph.getModuleById(resolveId);
        if (module) {
          // invalidate dependants
          server.moduleGraph.onFileChange(resolveId);
          // hot reload
          return [...modules, module ]
        }
        return modules;
      }
    },
  };
  return [squint];
}

This solved the issue with the tailwind HMR on a vanilla application. However, this approach does not work well with other plugins. For example the react plugin uses the vite/rollup createFilter. Internally this filter will never return true for virtual modules.

What next

Having interim physical files on the filesystem will be a lot easier than trying to do any tricks. We can go down two paths doing this, I would love input on which direction may be best:

  1. Start squints CLI in the background in the plugin to do the conversions (see thread). We can make it output to a particular directory and we can use the resolveId to resolve where vite should be checking for the files.
  2. We use the existing plugin and just persist the .cljs.jsx files to disk.

For option 1 this may be easier/better for supporting macros and a REPL, as we are resolving files we could also resolve external future sourcemaps with this approach. For option 2 we will still have to solve macros and how a potential future REPL would work.

borkdude commented 2 months ago

I vote for option 1 but we can expose programmatic access to the CLI so you don’t actually have to shell out.

https://www.michielborkent.nl https://www.eetvoorjeleven.nu

On Sun, 21 Apr 2024 at 16:40, Brandon Stubbs @.***> wrote:

I spent some time that day digging into it, but needed some more time to really go down the rabbit hole.

Below are my findings:

Tailwind changes are not being picked up as vite's internal postcss plugin is only checking changes for physical files. The module returns a resolveId of .cljs.jsx that would trick vite into thinking that there was a physical .cljs.jsx on the filesystem but it doesn't exist. This was the approach that worked the best when integrating into the other plugins (react/solid/qwik). Virtual Modules

I tried converting the plugin to emit virtual modules https://vitejs.dev/guide/api-plugin#virtual-modules-convention for the .cljs.jsx files. You can see the code below:

import { compileString } from "squint-cljs";import path, { dirname } from "path";import fs from "fs"; export default function viteSquint(opts = {}) { const squint = { name: "squint_compile", enforce: "pre", async load(id) { // load virtual files if (/^\0..cljs.jsx$/.test(id)) { // TODO: macros // TODO: squint source mapping const file = id.substring(1).replace(/.jsx$/, ""); const code = await fs.promises.readFile(file, "utf-8"); const compiled = compileString(code); return { code: compiled, map: null }; } }, resolveId(id, importer, options) { // importing cljs file if (!/\0./.test(id) && /.cljs$/.test(id)) { // make path relative if absolute if (id.startsWith("/")) { id = id.substring(1); } // if it's being imported from a virtual module, remove the \0 if (/^\0.*.cljs.jsx$/.test(importer)) { importer = importer.substring(1); } const absolutePath = path.resolve(dirname(importer), id); // return the virtual module id return "\0" + absolutePath + ".jsx"; }

  // importing another file from a module, vite doesn't resolve paths from
  // virtual modules, so we need to resolve it ourselves
  if (/^\0.*.cljs.jsx$/.test(importer)) {
    const realPath = dirname(importer.substring(1));
    const absolutePath = path.resolve(realPath, id);
    // we only return if the physical file exists, otherwise it's a library import
    if (fs.existsSync(absolutePath)) {
      return absolutePath;
    }
  }

},
handleHotUpdate({file, server, modules }) {
  if (/\.cljs$/.test(file)) {
    // this needs to be the same id returned by resolveId this is what
    // vite uses as the modules identifier
    const resolveId = "\0" + file + ".jsx";
    const module = server.moduleGraph.getModuleById(resolveId);
    if (module) {
      // invalidate dependants
      server.moduleGraph.onFileChange(resolveId);
      // hot reload
      return [...modules, module ]
    }
    return modules;
  }
},

}; return [squint];}

This solved the issue with the tailwind HMR on a vanilla application. However, this approach does not work well with other plugins. For example the react plugin uses the vite/rollup createFilter https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/src/index.ts#L96. Internally this filter will never return true https://github.com/rollup/plugins/blob/master/packages/pluginutils/src/createFilter.ts#L48 for virtual modules. What next

Having interim physical files on the filesystem will be a lot easier than trying to do any tricks. We can go down two paths doing this, I would love input on which direction may be best:

  1. Start squints CLI in the background in the plugin to do the conversions (see thread https://clojurians.slack.com/archives/C03U8L2NXNC/p1713557549985419). We can make it output to a particular directory and we can use the resolveId to resolve where vite should be checking for the files.
  2. We use the existing plugin and just persist the .cljs.jsx files to disk.

For option 1 this may be easier/better for supporting macros and a REPL, as we are resolving files we could also resolve external future sourcemaps with this approach. For option 2 we will still have to solve macros and how a potential future REPL would work.

— Reply to this email directly, view it on GitHub https://github.com/squint-cljs/vite-plugin-squint/issues/19#issuecomment-2068068478, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACFSBV5A6WU5VTFBICUATDY6PFULAVCNFSM6AAAAABGEGQIMCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYGA3DQNBXHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

martinklepsch commented 2 months ago

Option 1 sounds good to me as well. I'm wondering could the generated files be anywhere on the filesystem? Ideally I think the plugin shouldn't generate files into the source tree or something like that, maybe it should be just a tmp dir even.

I think avoiding the shell out is good as otherwise that will produce quite a bit of overhead I guess.

Just to recap what this would mean (this might be wrong)

  1. Vite sees a cljs file
  2. It uses a CLI-like API to compile the file into a specific directory
  3. It extends resolveId to resolve to that file on disk

PROFIT?! Am I getting this right?

I think it would make sense to not rely on squints watch as Vite already has the resolution built in and will only compile the files that are actually part of the module graph. But I'm not 100% sure if this might be required for resolveId to work correclty?

borkdude commented 2 months ago

Ideally I think the plugin shouldn't generate files into the source tree or something like that, maybe it should be just a tmp dir even.

squint.edn has :output-dir for this (and defaults to the same directory as the .cljs source)

It uses a CLI-like API to compile the file into a specific directory

Yes, the function squint.internal.cli/compile-file should be exposed for this (and made suitable for JS consumption) The :output-dir could be set here too, e.g. to a /tmp directory. Would be nice if this could be made configurable so I can use this for libraries that need outputted JS into a dist directory as well.

resolveId

I'm not sure what resolveId is about

borkdude commented 2 months ago

If resolveId is a function that sees an input .cljs file and should return a path into the :output-dir that could be exposed as well.

brandonstubbs commented 2 months ago

I think it would make sense to not rely on squints watch as Vite already has the resolution built in and will only compile the files that are actually part of the module graph. But I'm not 100% sure if this might be required for resolveId to work correctly?

Yes relying on Vite's watch would be ideal. As there are other things that Vite supports such as path aliasing. That is just an example that I am aware of.

So very similar to the steps mentioned above:

Will spend some time with some hardcoded files to see if this works in the plugin how we expect

borkdude commented 2 months ago

Cool, thanks for the experiment. Hardcoded experiment would be good, then I won't have to prematurely expose an API :)

brandonstubbs commented 2 months ago

I have created a POC that we can start working with https://github.com/squint-cljs/vite-plugin-squint/pull/20

The way it works:

  1. When vite calls options we could create an instance of the compiler
  2. When resolveId is called we do the following: i. If there is no .cljs extension in the import, we check if a cljs file exists with that extension and resolve it. ii. for .cljs files we return the path to the output-dir/filename.jsx. For example src/main.cljs will be squint_out/main.jsx iii. for imports that are files. Such as images, CSS, or json files. We return the absolute path as the jsx file in the output directory breaks relative paths. This is breaks in the current squint react example
  3. When load is called. We only compile if the file has changed to prevent an infinite loop and return the jsx from the compiled output
  4. When handleHotUpdate is called we invalidate using the resolveId that we returned before.

This works for both HMR and Tailwind HMR. This uses vites watch file logic, which helps us support path aliasing and json imports (and everything else that may be vite specific)

This does not work for macros, vite will also not ever call resolveId as it's not imported. Thus vite will never watch this file for changes. Perhaps the compiler could start in a watch mode only for macros?

borkdude commented 2 months ago

Cool, I'll have a look at this tomorrow