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

refactor!: better internal type safety for hooks #2995

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

MarshallOfSound
Copy link
Member

When combined with #2993 this makes writing forge configs in typescript a truly wonderful experience. Type hinting for each hook, arg names / types, etc.

If you want me to explain the generics stuff in shared-types that makes it all work you're gonna have to buy me a drink first 😅 .

This also changes the internal signature of Plugin.getHook(name) to Plugin.getHooks().name. This should be considered a breaking change I guess but I've taken the opportunity to update all our built-in plugins to the new syntax.

@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@8fa40d4). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 23f56d0 differs from pull request most recent head 95afad1. Consider uploading reports for the commit 95afad1 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2995   +/-   ##
=======================================
  Coverage        ?   72.90%           
=======================================
  Files           ?       67           
  Lines           ?     2185           
  Branches        ?      438           
=======================================
  Hits            ?     1593           
  Misses          ?      392           
  Partials        ?      200           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fa40d4...95afad1. Read the comment docs.

@MarshallOfSound MarshallOfSound requested a review from a team October 26, 2022 10:20
@MarshallOfSound MarshallOfSound force-pushed the better-hook-type-safety branch 3 times, most recently from ca1f41e to dfcaadc Compare October 26, 2022 19:48
@MarshallOfSound MarshallOfSound force-pushed the better-hook-type-safety branch from dfcaadc to 95afad1 Compare October 26, 2022 20:17
@MarshallOfSound MarshallOfSound merged commit 12e1af6 into main Oct 26, 2022
@MarshallOfSound MarshallOfSound deleted the better-hook-type-safety branch October 26, 2022 21:35
@gigantz
Copy link

gigantz commented Oct 30, 2022

Hey @MarshallOfSound, thanks for changes 👍
But it seems like this.isProd = true missing in prePackage hook ?

or there is a new approach to pass it

@MarshallOfSound
Copy link
Member Author

@gigantz What do you mean this.isProd = true? Not sure what you're referring to

@gigantz
Copy link

gigantz commented Oct 30, 2022

Check this line.
electron-forge package should set prod mode, but in the latest version it doesn't.

@MarshallOfSound
Copy link
Member Author

@gigantz Fixed in #3021

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.

3 participants