Skip to content

Adds mode to the zip filenames #1605

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

Closed
wants to merge 1 commit into from
Closed

Conversation

sleekslush
Copy link
Contributor

Overview

Manual Testing

Related Issue

This PR closes #<issue_number>

Copy link

netlify bot commented Apr 22, 2025

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit 6080c6b
🔍 Latest deploy log https://app.netlify.com/sites/creative-fairy-df92c4/deploys/6807e4b5b1a9180008c23f6a
😎 Deploy Preview https://deploy-preview-1605--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.

@Tensai75
Copy link
Contributor

I vote against this PR. mode should not be included in the default template for the zip file names.
There is aways the possibility to add custom artifactTemplate and sourcesTemplate to the wxt.config.ts if someone wants this.

@sleekslush
Copy link
Contributor Author

I vote against this PR. mode should not be included in the default template for the zip file names. There is aways the possibility to add custom artifactTemplate and sourcesTemplate to the wxt.config.ts if someone wants this.

This was done to keep parity with the default behavior of the output directory. There's a quick chat about it here too https://discord.com/channels/1212416027611365476/1224507127863967784/1359972453778067738

@Tensai75
Copy link
Contributor

Tensai75 commented Apr 25, 2025

Well, then please let me add my two cents to this discussion:
I strongly believe that default templates/settings should reflect the simplest use case that the majority of users would use/prefer. If more than 50% of users change the default templates/settings, they are poorly chosen.
And I would further argue that the majority of users only target one manifest mode per browser. In such a use case, most users would probably consider the mode to be irrelevant information that would only unnecessarily lengthen the name of the zip file.
I would even go so far as to suggest changing the default setting for the build folders to {{browser}} only, rather than changing the names of the zip files.

Anyway, this is just my opinion. But in any case, this PR is a breaking change that should be labelled as such.

@sleekslush
Copy link
Contributor Author

I would even go so far as to suggest changing the default setting for the build folders to {{browser}} only, rather than changing the names of the zip files.

That's the way it was before 0.20.0. The change is here https://github.com/wxt-dev/wxt/pull/1086/files#diff-ff0465c3a486d3ba187204149a25fc8f632c44d65da356dc04c0f2b268a71506

My entire goal is to simply keep them consistent, hence the change.

Anyway, this is just my opinion. But in any case, this PR is a breaking change that should be labelled as such.

Sure.

sourcesTemplate: '{{name}}-{{version}}-sources.zip',
artifactTemplate: '{{name}}-{{version}}-{{browser}}.zip',
sourcesTemplate: '{{name}}-{{version}}-{{browser}}-{{mode}}-sources.zip',
artifactTemplate: '{{name}}-{{version}}-{{browser}}-{{mode}}.zip',
Copy link
Contributor

@Tensai75 Tensai75 Apr 25, 2025

Choose a reason for hiding this comment

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

I have to admit I misunderstood the PR at first and thought it was the {{manifestVersion}} that should be added to the filenames, but it's the production mode.
Ok, then I agree with this PR in principle, because I too like consistency. But the outDirTemplate uses {{modeSuffix}} (‘-dev’ for development, ‘’ for production):

const outDirTemplate = (
mergedConfig.outDirTemplate ??
`${browser}-mv${manifestVersion}${modeSuffix}`
)

So we should use {{modeSuffix}} in artifactTemplate and sourcesTemplate as well and not add {{browser}} to sourcesTemplate (the source zip is exactly the same for all browsers and should be generated only once anyway):

sourcesTemplate: '{{name}}-{{version}}-sources{{modeSuffix}}.zip',
artifactTemplate: '{{name}}-{{version}}-{{browser}}{{modeSuffix}}.zip',

This would for development result in:

extensionname-1.0.0-chrome-dev.zip
extensionname-1.0.0-firefox-dev.zip
extensionname-1.0.0-sources-dev.zip

and for production in:

extensionname-1.0.0-chrome.zip
extensionname-1.0.0-firefox.zip
extensionname-1.0.0-sources.zip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm already handling this all in my config, so I'll just close this PR. Looks like you have another PR related to these templates, so I'll let you handle whatever you think will work best in the current state of things.

@sleekslush sleekslush closed this Apr 25, 2025
@aklinker1
Copy link
Collaborator

sourcesTemplate: '{{name}}-{{version}}-sources{{modeSuffix}}.zip'
artifactTemplate: '{{name}}-{{version}}-{{browser}}{{modeSuffix}}.zip'

Yes, we should do this in the next major version.

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