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

[BUG]: Doesn't appear to work in dev mode #1

Open
2 tasks done
auctumnus opened this issue May 14, 2021 · 12 comments
Open
2 tasks done

[BUG]: Doesn't appear to work in dev mode #1

auctumnus opened this issue May 14, 2021 · 12 comments
Assignees
Labels
bug Something isn't working pinned Prevent stale on pinned items

Comments

@auctumnus
Copy link

Prerequisites

Description

Plugin doesn't appear to work in dev mode at all. Takes another ~15 seconds than usual to start the dev server, and 62 (!) warnings pop up reading:
[plugin:vite-plugin-favicon] context method emitFile() is not supported in serve mode. This plugin is likely not vite-compatible. (x62)

Steps to Reproduce

  1. Clone the reproduction repo (https://github.com/auctumnus/vite-plugin-favicon-repro)
  2. Install dependencies (yarn)
  3. Run yarn dev to start the dev server
  4. See errors in console

Expected behavior:

I expected the favicons to be added, and for it to start without errors.

Actual behavior:

No favicon is added to the page, and Vite takes around 15 seconds extra to start, as well as emitting the warnings as above.

Reproduces how often:

Every time I've tried to reproduce it, it's worked.

@auctumnus auctumnus added the bug Something isn't working label May 14, 2021
@auctumnus
Copy link
Author

Notably, I just checked - it does modify the HTML file correctly, but the actual image files fail to be generated in dev mode - which is, I assume, a problem with emitFile().

@josh-hemphill
Copy link
Owner

It actually seems that I overlooked quite a lot. The favicons package doesn't currently allow me to provide updated file paths, so most of the paths in the manifests and browserconfig are wrong. I'm looking through the code of other asset-producing vite plugins to see how vite expects the assets to be added and it seems like's going to take a little while to implement.

@khalwat
Copy link

khalwat commented Jun 7, 2021

It actually seems that I overlooked quite a lot. The favicons package doesn't currently allow me to provide updated file paths, so most of the paths in the manifests and browserconfig are wrong. I'm looking through the code of other asset-producing vite plugins to see how vite expects the assets to be added and it seems like's going to take a little while to implement.

You should be able to control the paths via the favicons.path setting. For example, with these settings:

    ViteFaviconsPlugin({
      inject: false,
      logo: 'favicon_src.png',
      favicons: {
        path: './output/'
      }

browserconfig.xml will look like this:

<?xml version="1.0" encoding="utf-8"?>
<browserconfig>
    <msapplication>
        <tile>
            <square70x70logo src="output/mstile-70x70.png"/>
            <square150x150logo src="output/mstile-150x150.png"/>
            <wide310x150logo src="output/mstile-310x150.png"/>
            <square310x310logo src="output/mstile-310x310.png"/>
            <TileColor>#fff</TileColor>

        </tile>

    </msapplication>

</browserconfig>

@khalwat
Copy link

khalwat commented Jun 7, 2021

Notably, I just checked - it does modify the HTML file correctly, but the actual image files fail to be generated in dev mode - which is, I assume, a problem with emitFile().

I'm going to look into a PR where it will not attempt to emit a file if the command isn't build, which will effectively cause the slowdown to go away, as well as the errors.

It won't change the fact that the assets won't be built, but I'm less concerned about that in local dev.

It also is something that I can look into implementing in the future, I just want to triage it so it works for now.

@khalwat
Copy link

khalwat commented Jun 9, 2021

PR made: #4

  • causes the plugin to only emit files if command === 'build'
  • Adjusts favicons.path to match viteConfig.base + viteConfig.build.assetsDir

Also includes: #2

@josh-hemphill
Copy link
Owner

Sorry I haven't had a chance to address the PRs yet, or really get it this package working in general. I'm in the middle of a job transition that has resulted in all the projects that I had that were going to be using this getting canceled. And I don't entirely remember the options I had built in already to know if the PRs need to have explicit options added to control that behavior.

Another reason it's been a while since I've pushed is that I've run into the limitations of the favicons library, the only way to dynamically change the file paths after the icons are generated is to regenerate the icons completely, so I've been working on my own favicon library that's designed from the ground up to be used as a library, unlike favicons which is primarily a cli tool and doesn't give any access to its internals.

@khalwat
Copy link

khalwat commented Jun 14, 2021

Cool @josh-hemphill no worries, OSS shouldn't be a lodestone!

Would you be okay with me publishing my fork on npm with the name vite-plugin-favicon2 and crediting you? Reason is I'd like to use it in a project that runs in Docker... and I don't want to have to add git to the image just to pull from my fork's branch :)

Regarding the paths, my patch is an incomplete solution, but I don't really care about the favicons in local dev anyway, mostly I just care that they work properly in the production builds.

@josh-hemphill
Copy link
Owner

Sure thing.
Hopefully I'll be able to get back to this once I have time to spin up some vite-based personal projects to test it in.

@stale

This comment has been minimized.

@stale stale bot added the stale label Aug 13, 2021
@stale stale bot closed this as completed Aug 20, 2021
@josh-hemphill josh-hemphill reopened this Sep 16, 2021
@stale stale bot removed the stale label Sep 16, 2021
@josh-hemphill josh-hemphill added the pinned Prevent stale on pinned items label Sep 16, 2021
@khalwat
Copy link

khalwat commented Nov 13, 2021

I did end up publishing a patched up version of this as vite-plugin-favicon2:

https://github.com/khalwat/vite-plugin-favicon

@alexey-sh
Copy link

@josh-hemphill Do I understand correctly that the repo is abandoned?

@josh-hemphill
Copy link
Owner

Not necessarily. I've finally got a chance to use vite tooling again at my work, so I might have a chance to rewrite this soon, though I'll probably focus on my CSP plugin first since IMO, this plugin is a minor convenience over using something like realfavicongenerator.net

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pinned Prevent stale on pinned items
Projects
None yet
Development

No branches or pull requests

4 participants