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

feat: Auto Icons Module #851

Merged
merged 8 commits into from
Jul 26, 2024
Merged

feat: Auto Icons Module #851

merged 8 commits into from
Jul 26, 2024

Conversation

Timeraa
Copy link
Contributor

@Timeraa Timeraa commented Jul 24, 2024

Adds a new module to wxt to automatically generate icons for the manifest icons field using sharp

Closes #850

For the structure (package.json, tsconfig) I copied the one from the vue module, had to add a build config for fs-extra

Copy link

netlify bot commented Jul 24, 2024

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit 6113233
🔍 Latest deploy log https://app.netlify.com/sites/creative-fairy-df92c4/deploys/66a3283885798800082172f2
😎 Deploy Preview https://deploy-preview-851--creative-fairy-df92c4.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Timeraa
Copy link
Contributor Author

Timeraa commented Jul 24, 2024

@aklinker1 I noticed a small issue with the wxt module system, is there a way to get the module options in hooks? Because right now you would need to restart the whole process in order for the options to update.

Also if you want me to update any documentation, do let me know where

@aklinker1
Copy link
Collaborator

aklinker1 commented Jul 24, 2024

Are you saying if someone saves the wxt.config.ts file with new options, they aren't updated? Hmmm... Yeah I can see why that would be the case. Let me think on the best way to fix it.

@Timeraa
Copy link
Contributor Author

Timeraa commented Jul 24, 2024

Are you saying if someone saves the wxt.config.ts file with new options, they aren't updated? Hmmm... Yeah I can see why that would be the case. Let me think on the best way to fix that.

Yep exactly that! It totally makes sense why but probably something to think about!

Copy link
Collaborator

@aklinker1 aklinker1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, just a couple of small changes.

What are your thoughts on the module- prefix for the package names? I wanted it to be consistent, but now I'm regretting it... @wxt-dev/auto-icons sounds good, @wxt-dev/module-auto-icons seems redundant 😵‍💫

packages/module-auto-icons/package.json Outdated Show resolved Hide resolved
packages/module-auto-icons/build.config.ts Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jul 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.06%. Comparing base (2b6ff8d) to head (6113233).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #851      +/-   ##
==========================================
- Coverage   82.17%   82.06%   -0.12%     
==========================================
  Files         113      113              
  Lines        6323     6311      -12     
  Branches     1029     1029              
==========================================
- Hits         5196     5179      -17     
- Misses       1113     1118       +5     
  Partials       14       14              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aklinker1
Copy link
Collaborator

aklinker1 commented Jul 25, 2024

Looks like the generated icons aren't added to the internal output object, and are causing some issues:

Screenshot 2024-07-25 at 10 10 22 AM

And the icons don't show up in the build output.

Screenshot 2024-07-25 at 10 11 42 AM

Might need to add some more hooks to WXT to fix these issues.

@aklinker1
Copy link
Collaborator

aklinker1 commented Jul 25, 2024

For example, a prepare:publicPaths hook here to fix the browser.runtime.getURL issue.

Edit: I added the prepare:publicPaths hook in #858. Merge this branch with main to get it!

For the build summary issue... I think we can just add the icon files to the output.publicAssets file in the build:done hook.

wxt.hooks.hook("build:done", (wxt, output) => {
  output.publicAssets.push({ type: "asset", file: "icons/16.png");
});

@Timeraa
Copy link
Contributor Author

Timeraa commented Jul 25, 2024

Code looks good, just a couple of small changes.

What are your thoughts on the module- prefix for the package names? I wanted it to be consistent, but now I'm regretting it... @wxt-dev/auto-icons sounds good, @wxt-dev/module-auto-icons seems redundant 😵‍💫

I can make it just auto-icons if ya want, just did fit for consistency haha

@Timeraa
Copy link
Contributor Author

Timeraa commented Jul 25, 2024

Oh gosh what did I do to the branch

@Timeraa
Copy link
Contributor Author

Timeraa commented Jul 25, 2024

Implemented the requested changes @aklinker1, I also updated the background.ts so that the /icons path is correct in the geturl

Copy link
Collaborator

@aklinker1 aklinker1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented the requested changes @aklinker1, I also updated the background.ts so that the /icons path is correct in the geturl

Perfect, thanks. Everything looks good now.

I can make it just auto-icons if ya want, just did fit for consistency haha

Hmmmmmmmmmmmmmmmmmm... Yeah, let's rename it to just @wxt-dev/auto-icon

@aklinker1 aklinker1 changed the title feat(auto-icons): initial module feat: Auto Icons Module Jul 25, 2024
@aklinker1 aklinker1 merged commit a0971ca into wxt-dev:main Jul 26, 2024
16 checks passed
@Timeraa Timeraa deleted the auto-icons branch July 28, 2024 13:44
@aklinker1 aklinker1 mentioned this pull request Aug 16, 2024
30 tasks
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 this pull request may close these issues.

Automatically generate logos in correct sizes
2 participants