-
-
Notifications
You must be signed in to change notification settings - Fork 246
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: Ignore manifest.manifest_version
option and warn about incorrect usage
#1419
Conversation
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@wxt-dev/auto-icons
@wxt-dev/i18n
@wxt-dev/module-react
@wxt-dev/module-solid
@wxt-dev/module-vue
@wxt-dev/module-svelte
@wxt-dev/storage
@wxt-dev/unocss
wxt
commit: |
describe('manifest_version', () => { | ||
it('should ignore and log a warning when someone sets `manifest_version` inside the manifest', async () => { | ||
const buildOutput = fakeBuildOutput(); | ||
const expectedVersion = 2; | ||
setFakeWxt({ | ||
logger: mock(), | ||
config: { | ||
command: 'build', | ||
manifestVersion: expectedVersion, | ||
manifest: { | ||
manifest_version: 3, | ||
}, | ||
}, | ||
}); | ||
|
||
const { manifest } = await generateManifest([], buildOutput); | ||
|
||
expect(manifest.manifest_version).toBe(expectedVersion); | ||
expect(wxt.logger.warn).toBeCalledTimes(1); | ||
expect(wxt.logger.warn).toBeCalledWith( | ||
expect.stringContaining( | ||
'`manifest.manifest_version` config was set, but ignored', | ||
), | ||
); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new test failed before I implemented the fix in this PR.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1419 +/- ##
==========================================
- Coverage 81.40% 81.09% -0.32%
==========================================
Files 128 128
Lines 6272 6278 +6
Branches 1068 1070 +2
==========================================
- Hits 5106 5091 -15
- Misses 1151 1172 +21
Partials 15 15 ☔ View full report in Codecov by Sentry. |
Overview
manifest.manifest_version
is not how you configure the target manifest version. This PR ignores the option if set (previously, it overwrote the targetmanifest_version
, as described in the issue) and logs a warning telling you that it is configured wrong.Manual Testing
See unit tests. No manual testing required.
Related Issue
This PR closes #1388.