Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tailwind changes no longer detected #386

Closed
dvlden opened this issue Jan 29, 2024 · 12 comments · Fixed by #387
Closed

Tailwind changes no longer detected #386

dvlden opened this issue Jan 29, 2024 · 12 comments · Fixed by #387

Comments

@dvlden
Copy link
Contributor

dvlden commented Jan 29, 2024

Describe the bug

It's like it suddenly stopped working and it became a really painful DX. I am not really certain when did this happen, but I've been developing some new features on another branch, with so much time waste and pain. Every time I make some style changes, I must restart WXT in order to see them. Reloading the extension does not work either. Previously, this behaviour was only when I made changes in entry file popup/index.html for example, but now it is happening in all files.

To Reproduce

Will update issue once I figure out what the problem is.

  1. Pending...

Expected behavior

Should take effect once file is saved.

Video recording

wxt-tw.mp4

Environment


  System:
    OS: macOS 14.2.1
    CPU: (12) arm64 Apple M2 Max
    Memory: 55.40 GB / 96.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 21.2.0 - ~/.asdf/installs/nodejs/21.2.0/bin/node
    npm: 10.2.3 - ~/.asdf/plugins/nodejs/shims/npm
    pnpm: 8.14.3 - /opt/homebrew/bin/pnpm
    bun: 1.0.21 - /opt/homebrew/bin/bun
  Browsers:
    Brave Browser: 121.1.62.153
    Chrome: 121.0.6167.85
    Edge: 121.0.2277.83
    Firefox Nightly: 124.0a1
    Safari: 17.2.1
    Safari Technology Preview: 17.4
  npmPackages:
    wxt: ^0.13.0 => 0.13.5

Additional context

I tested the TW example, saw it's broken, but HMR works. Going to fix the template and then hunt down why it isn't working for me anymore.

@dvlden dvlden added the pending-triage Someone (usually a maintainer) needs to look into this to see if it's a bug label Jan 29, 2024
@aklinker1
Copy link
Collaborator

I see you're working on the reproduction, but I just wanted to say, I like the gradient border and logo you've got, it looks very crisp 😎

@dvlden
Copy link
Contributor Author

dvlden commented Jan 29, 2024

I see you're working on the reproduction, but I just wanted to say, I like the gradient border and logo you've got, it looks very crisp 😎

Yeah, I am trying to reproduce on my own project. Go back a handful of commits to the point where it worked and then see how/where could I fu*k it up.

Thanks, we could probably use something similar (animated border) for WXT examples too. 💯

@aklinker1
Copy link
Collaborator

From what I can tell looking at the example, the tailwind CSS file doesn't contain all the classes, just the ones present at build time. This is the usual behavior for production, obviously we want to strip out all the classes that aren't used, but in development, I thought tailwind's CSS file was populated with all it's classes...

@aklinker1
Copy link
Collaborator

OK, so the problem is that when we render the HTML file to the file system (so that Chrome doesn't say "ERR can't find popup.html for action.default_popup" while loading the extension), the assets should be loaded from localhost, not from the extension's directory.

This is because the code WXT uses to load ignores TSConfig path aliases 🤦 . The condition on line 60 is just looking for relative paths, and doesn't work with something like ~/assets/tailwind.css 🤦

const pointToDevServer = (
querySelector: string,
attr: string,
): void => {
document.querySelectorAll(querySelector).forEach((element) => {
const src = element.getAttribute(attr);
if (!src) return;
if (isAbsolute(src)) {
element.setAttribute(attr, server.origin + src);
} else if (src.startsWith('.')) {
const abs = resolve(dirname(id), src);
const pathname = relative(config.root, abs);
element.setAttribute(attr, `${server.origin}/${pathname}`);
}
});
};
pointToDevServer('script[type=module]', 'src');
pointToDevServer('link[rel=stylesheet]', 'href');

@aklinker1
Copy link
Collaborator

aklinker1 commented Jan 29, 2024

In the Tailwind example:

With ~/assets/main.css With ../../assets/main.css
Screenshot 2024-01-29 at 4 43 56 PM Screenshot 2024-01-29 at 4 43 33 PM

@aklinker1
Copy link
Collaborator

aklinker1 commented Jan 29, 2024

To fix this:

  1. In wxt/src/core/builders/vite/plugins/devHtmlPrerender.ts, we need to grab the resolve.alias option from the configResolved hook and store it in a varaible.
  2. Then, in the pointToDevServer in the same file, we need to attempt to resolve the path using the aliases. I don't know how to do this off the top of my head... Maybe @rollup/plugin-alias (the plugin vite uses to make it's resolve.alias work) provides a helper function we can use? Or we can just do simple str.replace? Like I said, not sure.

@dvlden
Copy link
Contributor Author

dvlden commented Jan 29, 2024

Yeah, I am at that point now. Just looking at ~/assets/app.css in my codebase.
I'm sorry, I do not follow the codebase snippet from above.

How the heck did you figure this so quickly? I went 30 commit behind, thinking it could be the src/public directory changes in the config.

@aklinker1
Copy link
Collaborator

I can handle this, but I'm busy until tomorrow after noon, so it will be a while.

For now, use relative paths instead of ~ or @ aliases.

@dvlden
Copy link
Contributor Author

dvlden commented Jan 29, 2024

Given the input, I'll try to add make this and see if it works.
I read it once, sounds simple, I read it twice, sounds complex. But I'll give it a shot.

If I fail, I'll simply wait for your solution. No rush. I developed for over a week with no tailwind styles being applied 🤣

@dvlden
Copy link
Contributor Author

dvlden commented Jan 29, 2024

You already have aliases in the config argument:

    alias: {
      '@': '/Users/nn/Projects/Personal/ultrawideo-v3/src',
      '~': '/Users/nn/Projects/Personal/ultrawideo-v3/src',
      '@@': '/Users/nn/Projects/Personal/ultrawideo-v3',
      '~~': '/Users/nn/Projects/Personal/ultrawideo-v3'
    },

Identical to the ones from the configResolved hook:

    {
      find: '@',
      replacement: '/Users/nn/Projects/Personal/ultrawideo-v3/src'
    },
    {
      find: '~',
      replacement: '/Users/nn/Projects/Personal/ultrawideo-v3/src'
    },
    {
      find: '@@',
      replacement: '/Users/nn/Projects/Personal/ultrawideo-v3'
    },
    {
      find: '~~',
      replacement: '/Users/nn/Projects/Personal/ultrawideo-v3'
    },

So unless we also need those for something:

    {
      find: 'webextension-polyfill',
      replacement: '/Users/nn/Projects/Personal/ultrawideo-v3/browser.ts'
    },
    {
      find: '@wxt/reload-html',
      replacement: '/Users/nn/Projects/Personal/ultrawideo-v3/node_modules/wxt/dist/virtual/reload-html.js'
    },
    {
      find: /^\/?@vite\/env/,
      replacement: '/@fs/Users/nn/Projects/dvlden-wxt/node_modules/.pnpm/vite@5.0.12_@types+node@20.10.3_sass@1.69.5/node_modules/vite/dist/client/env.mjs'
    },
    {
      find: /^\/?@vite\/client/,
      replacement: '/@fs/Users/nn/Projects/dvlden-wxt/node_modules/.pnpm/vite@5.0.12_@types+node@20.10.3_sass@1.69.5/node_modules/vite/dist/client/client.mjs'
    }

I'll assume we can use config.alias object instead? So sounds like we need just another condition that checks for alias? Will test this now.

@aklinker1
Copy link
Collaborator

aklinker1 commented Jan 30, 2024

Yeah, using InternalConfig.alias should be fine... If someone wants to add custom aliases, that's where they would add it.

@aklinker1
Copy link
Collaborator

Released in https://github.com/wxt-dev/wxt/releases/tag/v0.15.2

@aklinker1 aklinker1 removed the pending-triage Someone (usually a maintainer) needs to look into this to see if it's a bug label Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants