error on code reload

[PatrykKuniczak]

Describe the bug
Sometimes when i add some more code, e.g 50 lines, then i click inside plugin in chrome, and then i have error(scren shot section):

And i have this maybe once per 10/15 reloads.

Good to know is, i don't close plugin, popup is all time open, and after edit code i click inside plugin.

Almost all times works well, but sometimes don't want to reload.

Maybe it's related to #157

PS: When project grow up, then i one line change, may lead to an error.

To Reproduce
Steps to reproduce the behavior:

1. Clone and follow all steps for this repo reproduct.
2. Create e.g 1 custm component
3. Try to edit it, or paste several times in Popup.tsx

Screenshots

For english: Can't access file.
File could be moved. Editted or removed.

PS: This info is misleading, because user see, he have some error in plugin, but everything works fine.

I think this console.warn which display that, should be e.g console.info or sth like that, what don't be displayed as error in extenstions section.

Desktop (please complete the following information):

• OS: Window 11
• Browser: chrome latest
• Node Version: 18.17.1

[Jonghakseo]

I'll take a look. :)

[PatrykKuniczak]

@Jonghakseo Any new info? :)

[Jonghakseo]

@PatrykKuniczak

Yeah, I can't reproduce it very well. :(

#157 has been resolved, so I think it might have been resolved together, please check.

[PatrykKuniczak]

@PatrykKuniczak

Yeah, I can't reproduce it very well. :(

#157 has been resolved, so I think it might have been resolved together, please check.

Ok, after work i will take a look :)

[PatrykKuniczak]

@Jonghakseo It's still occur

As you can see, it's default view

And i'm copying several times this switch theme button.
I have all time open plugin in chrome.

And then after hmr build next time module, and it's finished.

Then i have this in chrome:

After click outside(close plugin) and click again, to open, i have proper view:

[Jonghakseo]

@PatrykKuniczak

Ok... It's weired...
Can u reproduce this case on video?

2023-10-15.4.13.24.mov

2023-10-15.4.12.42.mov

2023-10-15.4.17.37.mov

[PatrykKuniczak]

As you can see i have it, still (I was pulled all new commits from main)

And for be sure, I turned it off all other plugins, but the bug still occur.

I see you're using Mac, i have this bug on Windows.

Bug.mp4

This errors is new, i don't see it before:

[bambooom]

Hi, it there any progress on this issue? I also has the same error for twind

[PatrykKuniczak]

For me it's now ocurr almost everytime

[PatrykKuniczak]

@Jonghakseo any moves forward?

[vms-code]

@bambooom @PatrykKuniczak the error you are referring to happens because the content page uses a shadow DOM and twind functions. The shadow DOM separates the environment from the main DOM, this breaks the CSS, helpful link:
ShadowDom doc

So the way they make the CSS work is by adding the following function on the src/pages/content/components/Demo/index.tsx page:
attachTwindStyle(rootIntoShadow, shadowRoot);

This function creates a new CSSStyleSheet(), then uses the twind function to add tailwind class rules to the stylesheet and finally uses the: documentOrShadowRoot.adoptedStyleSheets = [sheet.target]; line which injects the CSS rules into the shadow DOM.

if you want to use custom css rules instead of twind rules you can change the code to this:

export function attachTwindStyle<T extends { adoptedStyleSheets: unknown }>(
  observedElement: Element,
  documentOrShadowRoot: T,
) {
  //const sheet = cssom(new CSSStyleSheet());
  //const tw = twind(config, sheet);
  //observe(tw, observedElement);
  
  const customSheet = new CSSStyleSheet();
  
  // this are the same rules you would put in a normal .css file
  const cssRules = " body { background: #000; } .myClass { color: #fff; } ..." 
  
  customSheet.replaceSync(cssRules);
  documentOrShadowRoot.adoptedStyleSheets = [customSheet];
}

other useful link: replaceSync

[PatrykKuniczak]

background: #000; } .myClass { color: #fff; } ..." \n \n customSheet.replaceSync(cssRules

I will take a look on that, i remove twid from my project, cause i was using mui with styled components, solution for this repo is probably remove twid completely, and style with pure tailwind, if the twid don't want to work in content page

[Jonghakseo]

@Jonghakseo any moves forward?

Nope.

[Jonghakseo]

background: #000; } .myClass { color: #fff; } ..." \n \n customSheet.replaceSync(cssRules

I will take a look on that, i remove twid from my project, cause i was using mui with styled components, solution for this repo is probably remove twid completely, and style with pure tailwind, if the twid don't want to work in content page

I agree. I'm going to remove the twind this weekend, and I'm thinking of configuring the solution that applies tailwind to be opt-in as a separate application method. I don't see any reason to put unnecessary code in for users who aren't going to use tailwind anyway.

[Jonghakseo]

• fix: remove twind from project #244

I don't think the removal of TWIND will cause the warning, but the reload issue that was initially raised as an issue is hard to reproduce.

Due to my situation, it is difficult to access the window PC development environment... I hope someone can help :(

[PatrykKuniczak]

@Jonghakseo
If i do

   socket.onerror = ev => {
    console.warn(ev);
  };

in initReloadClient.ts

then i can see an error, but it's sth strange:

Also still this error ocurr (more like info):

This info is cause the socket is disconnected for a second, and lose refrerence or STH like that, then can't woke up after all modules are reloaded, i try to solve this, but i don't create this hot module, and i must see how it's built.

[PatrykKuniczak]

@Jonghakseo Probably we have 2 separate issue, but maybe it's cause after i working on #233 i don't remove node_modules and there's some conflict, i remove entirely node_modules and dist and i will try to handle this again, it's really strange bug xD

[PatrykKuniczak]

@Jonghakseo It's sth with that i think:

But its really hard to debug

[PatrykKuniczak]

I don't see it before, it's probably after add pnpm to the project, but i'm not sure.

But before we can use this template with trouble for time to time, but not it's completely useless, break on each/one to 2/3 code change, and not like before, only popup stop, and i must open it again, now the entire server is down, and i must reload it, that's big issue.

[PatrykKuniczak]

@Jonghakseo on your PC it works properly?

[PatrykKuniczak]

@Jonghakseo It's sth with view.ts

when on

function reload(): void {
    pendingReload = false;
    window.location.reload();
  }

reload window, then

 function reloadWhenTabIsVisible(): void {
    !document.hidden && pendingReload && reload();
  }

  document.addEventListener('visibilitychange', reloadWhenTabIsVisible);

Handle visibility change, and run it again, and sth like loop is played here, maybe i'm wrong, but for me it's a possibly that, i must try to reproduce this, i will update here if i'll find sth new.

PS: Now i see the if (document.hidden) on onUpdate, check if window is visible, and don't refresh when it's hidden ...

[Jonghakseo]

I found a similar issue in your error logs, but I don't know the cause yet.
nodejs/node#31702

Never mind, this doesn't seem to be relevant.

[Jonghakseo]

@PatrykKuniczak

Since it's a permissions issue anyway, how about assigning higher permissions to that folder?

As far as I can tell from the error log, it seems to be a permissions issue, which is necessary to remove existing files during the build process.

[Jonghakseo]

@Jonghakseo on your PC it works properly?

I don't have window PC... 😞

[PatrykKuniczak]

@Jonghakseo Maybe you can try on virtual env, sandbox or sth like that

[PatrykKuniczak]

@PatrykKuniczak

Since it's a permissions issue anyway, how about assigning higher permissions to that folder?

As far as I can tell from the error log, it seems to be a permissions issue, which is necessary to remove existing files during the build process.

You have any idea how to do it?

[Jonghakseo]

@Jonghakseo Maybe you can try on virtual env, sandbox or sth like that

You're right! I will try it.

[PatrykKuniczak]

@Jonghakseo I see apple have sth like that: https://support.apple.com/pl-pl/guide/mac-help/mh11850/mac

Install windows is better option, if u can, cause sometimes on virtual env sth can work in differen way, but after all virtual env is better than nothing :)

[PatrykKuniczak]

@Jonghakseo I solve this part with permission,

chokidar.watch('src', { ignorePermissionErrors: true })

and also for 'dist', this error with crashing views scripts also ocurr..

If u have any idea feel free to push #256

[PatrykKuniczak]

@Jonghakseo Theres issue with this error:
#100

this race condition is the key of this problem.

I see the solution, cause delay isn't good solution, cause we can't be confident, how much it can take to reload entire code.

We must reformat that, and when 1 'watch' process works, don't run next for avoid race condition, when dev make change during rebuilding, this changes will be taken after the previouse process is done or kill the process when dev make change during rebuilding, sth like that nestjs have implemented.

I think in that case, we should listent on both sides od websockets, when client side, send complete message, then on server side, unlock next rebuild, this could work, but performance could be weak, for better performance we could, kill this process and run next one.

Then we can remove entirely delay.

Maybe if this don't work, we can use chokidar events to handle that, e.g. on 'unlink' block next process and on 'ready' check for new changes.

If you have any idea, let share with me :)

I will take a look for that tomorrow.

PS: If any of this solution will work, then we can remove closing ws between rebuildings, cause this is also problematic.

I think we should establish 1 connection on start script and this WSS should run till, we or error shut down script(read node server ;))

[Jonghakseo]

@Jonghakseo Theres issue with this error: #100

this race condition is the key of this problem.

I see the solution, cause delay isn't good solution, cause we can't be confident, how much it can take to reload entire code.

We must reformat that, and when 1 'watch' process works, don't run next for avoid race condition, when dev make change during rebuilding, this changes will be taken after the previouse process is done or kill the process when dev make change during rebuilding, sth like that nestjs have implemented.

I think in that case, we should listent on both sides od websockets, when client side, send complete message, then on server side, unlock next rebuild, this could work, but performance could be weak, for better performance we could, kill this process and run next one.

Then we can remove entirely delay.

Maybe if this don't work, we can use chokidar events to handle that, e.g. on 'unlink' block next process and on 'ready' check for new changes.

If you have any idea, let share with me :)

I will take a look for that tomorrow.

PS: If any of this solution will work, then we can remove closing ws between rebuildings, cause this is also problematic.

I think we should establish 1 connection on start script and this WSS should run till, we or error shut down script(read node server ;))

@PatrykKuniczak

Cool! I think the biggest problem is that the dist folder is detected by the file system.
I had an idea to send the build completion message from a custom plugin that is closely related to the build lifecycle, I'll try it.
You're really great.