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

Export MacPackager class and TypeScript types for codeSigningInfo in AfterPackContext #5972

Closed
jtbandes opened this issue Jun 18, 2021 · 12 comments · Fixed by #8153
Closed
Labels

Comments

@jtbandes
Copy link
Contributor

  • Version: 22.11.7
  • Electron Version: 13.1.2
  • Electron Type (current, beta, nightly): current
  • Target: macOS

An afterPack script that needs access to the temporary keychain created by MacPackager is able to access it via (await context.packager.codeSigningInfo.value).keychainFile. However, in TypeScript, context.packager is a generic PlatformPackager<any> so this requires awkward type casting (example in https://github.com/foxglove/studio/pull/1270).

It would be nice if the MacPackager class itself, or some sanctioned way of accessing the keychainFile, could be exported from the package's TypeScript definitions.

Workaround: the types for MacPackager can be imported via import type MacPackager from "app-builder-lib/out/macPackager"; but this seems to be an implementation detail. Guarding with context.packager instanceof MacPackager would require a full import from this non-standard path (not just import type) which seems like a bad idea if it's not an official export!

@mmaietta
Copy link
Collaborator

That's a great callout.
I'd be happy to accept a PR for this change! Instructions are fairly simple to set up a dev env 🙂
https://github.com/electron-userland/electron-builder/blob/master/CONTRIBUTING.md#to-setup-a-local-dev-environment

@jtbandes
Copy link
Contributor Author

Thanks, I can try to find a little spare time to work on this. Do you have any thoughts on the best approach? For example: exposing codeSigningInfo? on all packagers, exposing a packager "type" that can be used to safely typecast in TS, exporting the MacPackager class itself so clients can use instanceof, or some other solution?

@stale
Copy link

stale bot commented Aug 22, 2021

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the backlog label Aug 22, 2021
@stale stale bot closed this as completed Sep 22, 2021
@jtbandes
Copy link
Contributor Author

@mmaietta This issue is still relevant for us – if you have some guidance on these questions I'd be open to making a PR!

@mmaietta
Copy link
Collaborator

mmaietta commented Mar 14, 2024

I see two routes I suppose?

  • I export MacPackager, LinuxPackager, and WinPackager explicitly from app-builder-lib
  • OR
  • You continue to use it as an export directly via app-builder-lib/out/xxx.

Or am I misunderstanding your post?

If you're worried about the API changing, I have no intention of doing so. Additionally, if you were to explicitly pin the dependency version in your package.json, you wouldn't need to worry about it randomly breaking when an electron-builder update were to be released.

@jtbandes
Copy link
Contributor Author

Partly, I would like to avoid importing from app-builder-lib directly. Importing it directly does work, but causes trouble with Dependabot wanting to bump the version of app-builder-lib independently of the version of electron-builder, which sometimes leads to incompatibilities. That's why I was thinking it would be ideal if the types were exported from electron-builder (the package that users actually use directly).

@mmaietta
Copy link
Collaborator

Great feedback and clarification. I'll export the classes directly via the main electron-builder typings. PR #8153

@jtbandes
Copy link
Contributor Author

❤️ thanks! I guess I could’ve included that detail in the original ticket 😅

@jtbandes
Copy link
Contributor Author

Additionally, we are currently importing log and Arch from builder-util inside an afterPack script, and I’d love to be able to get rid of those imports too. Let me know if you have any advice for those (I don’t think they were exported elsewhere)

@mmaietta
Copy link
Collaborator

mmaietta commented Mar 25, 2024

Wait, just to confirm. Are you also looking for the AfterPackContext have an explicit typing as to what the packager is?

I'm wondering if we change the property signature to be a nested and have access to the specific Packager class based on which packager property is populated. Thoughts?

No matter what, I can't seem to be able to avoid having you do a force-cast in your code, as the packContext is heavily used elsewhere in the code for detecting platform and the like within other logic flows that don't have access to which type of packager is being returned. Still investigating further

  readonly packager: {
    packager: PlatformPackager<any>
    type: 'WinPackager' | 'MacPackager' | 'LinuxPackager'
  }

@mmaietta
Copy link
Collaborator

Additionally, we are currently importing log and Arch from builder-util inside an afterPack script

Also, Arch is already exported via electron-builder directly. I can update my PR to do the same for log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants