-
Notifications
You must be signed in to change notification settings - Fork 642
feat!: Allow manifest to extend another manifest #3802
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
Conversation
packages/snaps-utils/src/types.ts
Outdated
| /** | ||
| * A utility type that makes all properties of a type optional, recursively. | ||
| */ | ||
| type DeepPartial<Type> = { |
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.
Do we not have this somewhere already?
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.
Just one that works on objects in snaps-cli's test utils. I couldn't find a general one.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3802 +/- ##
=======================================
Coverage 98.33% 98.34%
=======================================
Files 422 422
Lines 12073 12127 +54
Branches 1875 1885 +10
=======================================
+ Hits 11872 11926 +54
Misses 201 201 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
| const mainManifest = await readJsonFile(manifestPath); | ||
| files.add(manifestPath); | ||
|
|
||
| if (!isPlainObject(mainManifest.result)) { |
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.
Do we care that these objects are plain? E.g. versus isObject?
Same question for a lot of this file.
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.
Not really, but unsure when to use isObject or isPlainObject or what the benefit is of one over the other 😅
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.
Updated to isObject.
Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
This is a follow up to #3793, which introduces custom manifests (e.g., one for production, one for development), allowing a custom manifest to extend another manifest. Right now, the entire production manifest would need to be copied to the development manifest. After this change it's possible to do the following:
With a base manifest like:
After which the manifests would be merged like this:
{ "proposedName": "My Snap", "other": "properties", "initialConnections": { "http://example.com": {} } }Right now this is mainly useful for local development, but may be used for bundling production Snaps in the future.
Breaking changes
checkManifestfunction now returnsExtendableSnapFiles, which contains anExtendableSnapManifest.Note
Enables manifest inheritance by allowing a manifest to
extendsanother, producing a merged manifest for tooling.loadManifestto recursively load/merge manifests (cycle detection), introduceExtendableManifest/ExtendableSnapFilesandDeepPartial; update schema to allowextends.manifest.mergedManifestand write fixes tomanifest.mainManifest;checkManifestnow returnsExtendableSnapFiles(BREAKING).SnapsWatchPlugintakesmanifestPathand watches all files fromloadManifest; config and snapshots updated./snap.manifest.jsonserves the merged manifest JSON; allowed static paths computed from merged manifest.bip32/snap.manifest.dev.jsonnowextendsbase manifest.Written by Cursor Bugbot for commit 4eabac7. This will update automatically on new commits. Configure here.