feat(nsis): add support for more Windows Installer options #9119
Conversation
🦋 Changeset detectedLatest commit: cf2ddfa The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| UNINSTALL_URL_HELP: options.uninstallUrlHelp || this.packager.info.metadata.homepage || undefined, | ||
| UNINSTALL_URL_INFO_ABOUT: options.uninstallUrlInfoAbout || this.packager.info.metadata.homepage || undefined, | ||
| UNINSTALL_URL_UPDATE_INFO: options.uninstallUrlUpdateInfo || this.packager.info.metadata.homepage || undefined, | ||
| UNINSTALL_URL_README: options.uninstallUrlReadme || this.packager.info.metadata.homepage || undefined, |
There was a problem hiding this comment.
I think undefined may have unexpected side effects. Note: defines.ENABLE_LOGGING_ELECTRON_BUILDER = null where null is used as a flag (I guess implicitly represents a boolean in the nsis scripts?)
Easy way around this - only set the defines var when provided a value
const { homepage } = this.packager.info.metadata
// `use` from builder-util IIRC
use(options.uninstallUrlHelp || homepage, (it) => defines.UNINSTALL_URL_HELP = it)
use(options.uninstallUrlInfoAbout || homepage, (it) => defines.UNINSTALL_URL_INFO_ABOUT = it)
use(options.uninstallUrlUpdateInfo || homepage, (it) => defines.UNINSTALL_URL_UPDATE_INFO = it)
use(options.uninstallUrlReadme || homepage, (it) => defines.UNINSTALL_URL_README = it)
There was a problem hiding this comment.
Also, wondering if we need to allow disabling the field from being set if the developer doesn't want their homepage to show there. Curious on your thoughts? Basically, uninstallUrlHelp?: string | false
There was a problem hiding this comment.
I don't think that's necessary. The only use case for defining the homepage is to show it somewhere, whether that's the npm registry or somewhere else, you want that homepage to be linked to the project.
Closes #9105