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

fix(hooks): move entrypoints:resolved hook and init entrypoints:paths hook #1292

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

abhigyantrips
Copy link

as mentioned in #1072, there's a requirement for a hook to manipulate entrypoint resolution before loading them.

  1. as there have been changes to the findEntrypoints() function since the issue, it's a bit outdated and the solution right now is to just move the entrypoints:resolved hook to before logging.
  2. as per a convo with @aklinker1 we should also introduce an entrypoints:paths hook to be able to manipulate the entrypointInfo variable.

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit 0dd7af1
🔍 Latest deploy log https://app.netlify.com/sites/creative-fairy-df92c4/deploys/67650606694483000849bd88
😎 Deploy Preview https://deploy-preview-1292--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.

@abhigyantrips
Copy link
Author

probably should change entrypointInfos to just infos.

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.

Nice, this is perfect!

Thoughts on renaming entrypoints:paths to entrypoints:found or entrypoints:discovered? This would better align with the noun:past-tense-verb naming convention we use for most of the other hooks. I realized entrypoints:paths wasn't a very good name after sleeping on it lol.

Copy link

pkg-pr-new bot commented Dec 19, 2024

Open in Stackblitz

@wxt-dev/auto-icons

npm i https://pkg.pr.new/@wxt-dev/auto-icons@1292

@wxt-dev/i18n

npm i https://pkg.pr.new/@wxt-dev/i18n@1292

@wxt-dev/module-solid

npm i https://pkg.pr.new/@wxt-dev/module-solid@1292

@wxt-dev/module-react

npm i https://pkg.pr.new/@wxt-dev/module-react@1292

@wxt-dev/module-svelte

npm i https://pkg.pr.new/@wxt-dev/module-svelte@1292

@wxt-dev/module-vue

npm i https://pkg.pr.new/@wxt-dev/module-vue@1292

@wxt-dev/storage

npm i https://pkg.pr.new/@wxt-dev/storage@1292

@wxt-dev/unocss

npm i https://pkg.pr.new/@wxt-dev/unocss@1292

wxt

npm i https://pkg.pr.new/wxt@1292

commit: 0dd7af1

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.18%. Comparing base (30b96ac) to head (0dd7af1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1292      +/-   ##
==========================================
+ Coverage   81.11%   81.18%   +0.06%     
==========================================
  Files         128      128              
  Lines        6280     6281       +1     
  Branches     1065     1064       -1     
==========================================
+ Hits         5094     5099       +5     
+ Misses       1171     1167       -4     
  Partials       15       15              

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

@abhigyantrips
Copy link
Author

entrypoints:found seems to stick well with entrypoints:grouped and entrypoints:resolved. will make those changes now.

any changes required to the addition in the docs?

@aklinker1
Copy link
Collaborator

aklinker1 commented Dec 20, 2024

any changes required to the addition in the docs?

Maybe a recipe on the modules page for adding an entrypoint using the new hook.

https://wxt.dev/guide/essentials/wxt-modules.html#recipes

@aklinker1
Copy link
Collaborator

aklinker1 commented Dec 20, 2024

Alright, thanks for adding this! The new hook will be very useful. Great quality PR too 👍

@aklinker1
Copy link
Collaborator

Hmm, not sure why those checks are failing... I'll get it sorted out and get the PR merged soon. Holidays and family in town starting tomorrow, so it might be a few days before I can look into this.

@aklinker1
Copy link
Collaborator

Looks like it's related to vite 6.0.4: #1293

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.

2 participants