marko-js / vite

A Marko plugin for Vite
MIT License
55 stars 8 forks source link

HMR does not work properly with class components #75

Closed UROjQ6r80p closed 5 months ago

UROjQ6r80p commented 1 year ago

Version: x.x.x

2.4.8

Details

HMR does not work properly with class components.

I use npx @marko/create -> vite-express -> npm install @marko/vite@latest

HMR does not work properly with class components. In vite-express starter, by default there is component app-button if i try to add random <span>test</span> to it, the console shows: [vite] hmr update /src/components/app-button/index.marko?marko-browser, /src/components/app-button/index.marko but component isn't reloaded correctly in the browser. It shows the outdated version even after manually refreshing the browser.

For components without class, for example app-layout it seems to always work, and then console shows [vite] page reload src/components/app-layout/index.marko

Expected Behavior

Component should be replaced to the new version

Actual Behavior

above.

Possible Fix

Additional Info ### Your Environment - Environment name and version (e.g. Chrome 39, node.js 5.4): - Operating System and version (desktop or mobile): - Link to your project: ### Steps to Reproduce 1. first... 2. 3. 4. ### Stack Trace
DylanPiercey commented 1 year ago

@UROjQ6r80p I can't seem to reproduce this myself. Could you share the result of running:

npm ls marko @marko/compiler @marko/vite vite

Also what operating system and browser you are using?

UROjQ6r80p commented 1 year ago

@DylanPiercey

Before I've tried the latest version of marko packages and vite. But it also does not work properly right after using npx @marko/create -> vite-express

Here is the result:

├── @marko/compiler@5.31.1
├─┬ @marko/express@2.1.0
│ └── marko@5.29.1 deduped
├─┬ @marko/vite@2.4.8
│ ├── @marko/compiler@5.31.1 deduped
│ └── vite@4.4.4 deduped
├─┬ marko@5.29.1
│ ├── @marko/compiler@5.31.1 deduped
│ └─┬ @marko/translator-default@5.29.1
│   ├── @marko/compiler@5.31.1 deduped
│   └── marko@5.29.1 deduped
└── vite@4.4.4

System: Windows 10. Browser: Brave, but also tried Chrome, Firefox, Opera.

Basically what I'm doing is constantly adding few letters in the button, then removing it, then adding, then removing, and it always shows outdated component.

https://github.com/marko-js/vite/assets/90694123/d6bfd008-c7ad-47e2-bc4b-1730fae386fb

I've tried to reproduce it on StackBlitz https://stackblitz.com/edit/github-fxezeu?file=src%2Fcomponents%2Fapp-button%2Findex.marko and it seems to work without problems so i guess it's related to Windows specifically.

Additionally, for example this url: http://localhost:3000/src/components/app-button/index.marko after I change the component and save file, this url always show up to date component. but this: http://localhost:3000/src/components/app-button/index.marko?marko-browser shows outdated version.

I tried to locally change this line at https://github.com/marko-js/vite/blob/main/src/index.ts

async load(id) {
        const query = getMarkoQuery(id);
        switch (query) {
          case serverEntryQuery: {
            entryIds.add(id.slice(0, -query.length));
            return null;
          }
          case browserEntryQuery:
          case browserQuery: {
            // COMMENTED THIS LINE return cachedSources.get(id.slice(0, -query.length)) || null;
            return virtualFiles.get(id) || null; // ADDED THIS
          }
        }

        return virtualFiles.get(id) || null;
      }

and after this change it seems to always show updated version, but i dont know how it affects the rest of the code.

danbarbarito commented 5 months ago

I'm running into this issue as well. Also on Windows.

danbarbarito commented 5 months ago

Actually @UROjQ6r80p I was just able to fix this. I turned off "linked" mode and it seemed to work now. At the root of my project, I have a vite.config.js with the following:

import { defineConfig } from "vite";
import marko from "@marko/run/vite";

export default defineConfig({
  plugins: [
    marko({
      linked: false,
    }),
  ],
});
DylanPiercey commented 5 months ago

@danbarbarito by turning linked: off you opt into exclusively server side rendering (or client rendering depending on how you configured vite).

There may well be a windows specific issue here, I need to take some time to check 😓. The mentioned line by @UROjQ6r80p https://github.com/marko-js/vite/blob/main/src/index.ts#L559 should be invalidated on change through vites watcher here https://github.com/marko-js/vite/blob/main/src/index.ts#L400

I wonder if it's got differently normalized file names here 🤔

DylanPiercey commented 5 months ago

@danbarbarito I think it may work if we change https://github.com/marko-js/vite/blob/main/src/index.ts#L400 to cachedSources.delete(normalizePath(filename));. Are you able to make that change in your node_modules and see if it works for you?

danbarbarito commented 5 months ago

@DylanPiercey My node_modules only has the dist directory available in @marko/vite. If you know of a quick way to pull down the source and use that instead please let me know. I'll try and figure it out in the meantime

DylanPiercey commented 5 months ago

@danbarbarito probably simplest to just edit the dist. Could you open node_modules/@marko/vite/dist/index.mjs and search for cachedSources.delete(filename)? It should be line 897

danbarbarito commented 5 months ago

@DylanPiercey That didnt work for me unfortunately 😢

DylanPiercey commented 5 months ago

Shoot. I'll have to get a windows vm setup again so I can test properly.

danbarbarito commented 5 months ago

Ok, I appreciate you looking into it. If its easier for us to get on a call and have me share my screen as you tell me the changes to make just let me know.

danbarbarito commented 5 months ago

Wait I messed up before! It works @DylanPiercey ! I guess I didn't restart Vite before or something. Sorry for the confusion.

DylanPiercey commented 5 months ago

Oh that's awesome. Allow me to do a quick release then!

DylanPiercey commented 5 months ago

Should be fixed in 4.1.7