-
Notifications
You must be signed in to change notification settings - Fork 229
Extensions: Compatibility guarantee #1732
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,288 @@ | ||
| import { | ||
| MESHERY_EXTENSION_CAPABILITIES, | ||
| MESHERY_EXTENSION_CONTRACT_VERSION, | ||
| MESHERY_EXTENSION_DEPRECATED_CAPABILITIES, | ||
| MESHERY_EXTENSION_EVENT, | ||
| MESHERY_EXTENSION_EVENT_TYPES, | ||
| MESHERY_EXTENSION_HOOKS, | ||
| describeInjectedCapabilityReport, | ||
| isInjectedCapabilityReportSatisfied, | ||
| isMesheryExtensionEvent, | ||
| reportInjectedCapabilities | ||
| } from '../actors/mesheryExtensionContract'; | ||
|
|
||
| /** A host bag that satisfies the contract exactly, built from the contract itself. */ | ||
| const satisfyingInjectProps = () => ({ | ||
| ...Object.fromEntries(MESHERY_EXTENSION_CAPABILITIES.map(key => [key, () => null])), | ||
| hooks: Object.fromEntries(MESHERY_EXTENSION_HOOKS.map(hook => [hook, () => null])) | ||
| }); | ||
|
|
||
| describe('Meshery extension contract — event literals', () => { | ||
| it('exposes a literal for every event type on the bus', () => { | ||
| // The union -> map completeness guard is compile-time; this asserts the | ||
| // runtime array stays in step with the map it is derived from. | ||
| expect([...MESHERY_EXTENSION_EVENT_TYPES].sort()).toEqual( | ||
| Object.values(MESHERY_EXTENSION_EVENT).sort() | ||
| ); | ||
| }); | ||
|
|
||
| it('pins the wire literals that cross the host boundary', () => { | ||
| // Hard-coded on purpose. These strings are the actual contract: a rename is | ||
| // only safe alongside a contract-version bump and a deprecation entry, and | ||
| // this test is what forces that conversation. Renaming | ||
| // OPEN_DESIGN_IN_KANVAS -> OPEN_DESIGN_IN_EXTENSION silently broke opening a | ||
| // design because nothing pinned them. | ||
| expect(MESHERY_EXTENSION_EVENT).toEqual({ | ||
| K8sContextsUpdated: 'K8S_CONTEXTS_UPDATED', | ||
| OpenViewScopedToDesign: 'OPEN_VIEW_SCOPED_TO_DESIGN', | ||
| OpenDesignInExtension: 'OPEN_DESIGN_IN_EXTENSION', | ||
| OpenViewInExtension: 'OPEN_VIEW_IN_EXTENSION', | ||
| MergeDesign: 'MERGE_DESIGN', | ||
| DispatchToMesheryStore: 'DISPATCH_TO_MESHERY_STORE', | ||
| FeatureRequiresUserAccount: 'FeatureRequiresUserAccount', | ||
| MissingPermission: 'MISSING_PERMISSION', | ||
| MissingCapability: 'MISSING_CAPABILITY' | ||
| }); | ||
| }); | ||
|
|
||
| it('recognises contract events and rejects unknown literals', () => { | ||
| expect( | ||
| isMesheryExtensionEvent({ | ||
| type: MESHERY_EXTENSION_EVENT.OpenDesignInExtension, | ||
| data: { designId: 'd', designName: 'n' } | ||
| }) | ||
| ).toBe(true); | ||
| // The pre-fix literal. A subscriber that receives this is talking to a host | ||
| // built against a different contract and must say so rather than no-op. | ||
| expect(isMesheryExtensionEvent({ type: 'OPEN_DESIGN_IN_KANVAS', data: {} })).toBe(false); | ||
| }); | ||
|
|
||
| it('rejects non-event values without throwing', () => { | ||
| expect(isMesheryExtensionEvent(null)).toBe(false); | ||
| expect(isMesheryExtensionEvent(undefined)).toBe(false); | ||
| expect(isMesheryExtensionEvent('OPEN_DESIGN_IN_EXTENSION')).toBe(false); | ||
| expect(isMesheryExtensionEvent({})).toBe(false); | ||
| }); | ||
|
|
||
| it('rejects a known literal carrying a payload this build cannot read', () => { | ||
| // The rename half of the same skew: the host keeps publishing a literal we | ||
| // recognise but renames the field inside it, so every subscriber reads | ||
| // undefined. Narrowing on the discriminant alone would call this valid and | ||
| // hand the subscriber a payload that throws on first property access. | ||
| const type = MESHERY_EXTENSION_EVENT.OpenDesignInExtension; | ||
|
|
||
| expect(isMesheryExtensionEvent({ type, data: null })).toBe(false); | ||
| expect(isMesheryExtensionEvent({ type })).toBe(false); | ||
| expect(isMesheryExtensionEvent({ type, data: {} })).toBe(false); | ||
| // `designId` renamed to `id` - recognised literal, unreadable payload. | ||
| expect(isMesheryExtensionEvent({ type, data: { id: 'd', designName: 'n' } })).toBe(false); | ||
| expect(isMesheryExtensionEvent({ type, data: { designId: 1, designName: 'n' } })).toBe(false); | ||
| }); | ||
|
|
||
| it('accepts a valid payload for every event literal', () => { | ||
| const valid: Record<(typeof MESHERY_EXTENSION_EVENT_TYPES)[number], unknown> = { | ||
| K8S_CONTEXTS_UPDATED: { selectedK8sContexts: ['ctx'] }, | ||
| OPEN_VIEW_SCOPED_TO_DESIGN: { designId: 'd', designName: 'n' }, | ||
| OPEN_DESIGN_IN_EXTENSION: { designId: 'd', designName: 'n' }, | ||
| OPEN_VIEW_IN_EXTENSION: { viewId: 'v', viewName: 'n' }, | ||
| MERGE_DESIGN: { id: 'i', name: 'n' }, | ||
| DISPATCH_TO_MESHERY_STORE: { type: 'redux/action' }, | ||
| FeatureRequiresUserAccount: { feature: 'f' }, | ||
| MISSING_PERMISSION: { keyId: 'k' }, | ||
| MISSING_CAPABILITY: { capabilityId: 'c' } | ||
| }; | ||
|
|
||
| for (const type of MESHERY_EXTENSION_EVENT_TYPES) { | ||
| expect(isMesheryExtensionEvent({ type, data: valid[type] })).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it('tolerates extra payload fields so additive changes stay compatible', () => { | ||
| // Only a rename or removal is a contract break. A newer host adding a field | ||
| // to a payload must not make an older bundle reject the whole event. | ||
| expect( | ||
| isMesheryExtensionEvent({ | ||
| type: MESHERY_EXTENSION_EVENT.MergeDesign, | ||
| data: { id: 'i', name: 'n', addedByANewerHost: true } | ||
| }) | ||
| ).toBe(true); | ||
| expect( | ||
| isMesheryExtensionEvent({ | ||
| type: MESHERY_EXTENSION_EVENT.DispatchToMesheryStore, | ||
| data: { type: 'redux/action', payload: { a: 1 } } | ||
| }) | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it('does not mistake an inherited Object member for a payload check', () => { | ||
| // The check table is a plain object, so an event whose `type` names something | ||
| // on Object.prototype must not resolve to an inherited function and pass. | ||
| expect(isMesheryExtensionEvent({ type: 'toString', data: {} })).toBe(false); | ||
| expect(isMesheryExtensionEvent({ type: 'constructor', data: {} })).toBe(false); | ||
| expect(isMesheryExtensionEvent({ type: 'hasOwnProperty', data: {} })).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Meshery extension contract — injected capabilities', () => { | ||
| it('reports a fully-satisfying host as satisfied', () => { | ||
| const report = reportInjectedCapabilities(satisfyingInjectProps()); | ||
|
|
||
| expect(report.missing).toEqual([]); | ||
| expect(report.missingHooks).toEqual([]); | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(true); | ||
| expect(describeInjectedCapabilityReport(report)).toBeNull(); | ||
| }); | ||
|
|
||
| it('names the capability a host stopped injecting', () => { | ||
| // This is the capabilitiesRegistry -> providerCapabilities incident in | ||
| // miniature: the host drops a key, every read yields undefined, and the | ||
| // failure used to surface as an unrelated crash far from the cause. | ||
| const injectProps = satisfyingInjectProps(); | ||
| delete (injectProps as Record<string, unknown>).providerCapabilities; | ||
|
|
||
| const report = reportInjectedCapabilities(injectProps); | ||
|
|
||
| expect(report.missing).toEqual(['providerCapabilities']); | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(false); | ||
| expect(describeInjectedCapabilityReport(report)).toContain('providerCapabilities'); | ||
| }); | ||
|
|
||
| it('names a missing nested hook', () => { | ||
| const injectProps = { ...satisfyingInjectProps(), hooks: { CAN: () => true } }; | ||
|
|
||
| const report = reportInjectedCapabilities(injectProps); | ||
|
|
||
| expect(report.missingHooks).toEqual(['useDynamicComponent', 'useFilterK8sContexts']); | ||
| expect(describeInjectedCapabilityReport(report)).toContain('useFilterK8sContexts'); | ||
| }); | ||
|
|
||
| it('does not double-report hooks when the whole hooks bag is absent', () => { | ||
| const injectProps = satisfyingInjectProps(); | ||
| delete (injectProps as Record<string, unknown>).hooks; | ||
|
|
||
| const report = reportInjectedCapabilities(injectProps); | ||
|
|
||
| expect(report.missing).toContain('hooks'); | ||
| expect(report.missingHooks).toEqual([]); | ||
| }); | ||
|
|
||
| it('flags deprecated aliases alongside their replacement without failing the check', () => { | ||
| const report = reportInjectedCapabilities({ | ||
| ...satisfyingInjectProps(), | ||
| capabilitiesRegistry: {}, | ||
| CapabilitiesRegistryClass: class {} | ||
| }); | ||
|
|
||
| expect(report.deprecated).toEqual([ | ||
| { provided: 'capabilitiesRegistry', replacement: 'providerCapabilities' }, | ||
| { provided: 'CapabilitiesRegistryClass', replacement: 'ProviderUiAccessControlClass' } | ||
| ]); | ||
| // Legacy bundles still depend on these, so their presence is informational. | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(true); | ||
| }); | ||
|
|
||
| it('does not report the host handshake field as an unknown key', () => { | ||
| // The host advertises `contractVersion` on the same bag. Reporting the | ||
| // host's own handshake field as unrecognized is noise, and noise is what | ||
| // trains people to stop reading the report. | ||
| const report = reportInjectedCapabilities({ | ||
| ...satisfyingInjectProps(), | ||
| contractVersion: MESHERY_EXTENSION_CONTRACT_VERSION | ||
| }); | ||
|
|
||
| expect(report.unrecognized).toEqual([]); | ||
| }); | ||
|
|
||
| it('reports keys this build has never heard of as unrecognized', () => { | ||
| const report = reportInjectedCapabilities({ | ||
| ...satisfyingInjectProps(), | ||
| someCapabilityFromANewerHost: () => null | ||
| }); | ||
|
|
||
| expect(report.unrecognized).toEqual(['someCapabilityFromANewerHost']); | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(true); | ||
| }); | ||
|
|
||
| it('treats a null or undefined bag as every capability missing', () => { | ||
| for (const bag of [null, undefined]) { | ||
| const report = reportInjectedCapabilities(bag); | ||
| expect(report.missing).toEqual([...MESHERY_EXTENSION_CAPABILITIES]); | ||
| expect(describeInjectedCapabilityReport(report)).toContain('contract'); | ||
| } | ||
| }); | ||
|
|
||
| it('keeps every deprecated alias pointing at a real capability or an outright removal', () => { | ||
| for (const replacement of Object.values(MESHERY_EXTENSION_DEPRECATED_CAPABILITIES)) { | ||
| if (replacement === null) continue; | ||
| expect(MESHERY_EXTENSION_CAPABILITIES).toContain(replacement); | ||
| } | ||
| }); | ||
|
|
||
| it('never lists a deprecated alias as a current capability', () => { | ||
| for (const alias of Object.keys(MESHERY_EXTENSION_DEPRECATED_CAPABILITIES)) { | ||
| expect(MESHERY_EXTENSION_CAPABILITIES).not.toContain(alias); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('Meshery extension contract — version', () => { | ||
| it('is a positive integer hosts and extensions can compare', () => { | ||
| expect(Number.isInteger(MESHERY_EXTENSION_CONTRACT_VERSION)).toBe(true); | ||
| expect(MESHERY_EXTENSION_CONTRACT_VERSION).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it('treats a matching advertised version as compatible', () => { | ||
| const report = reportInjectedCapabilities({ | ||
| ...satisfyingInjectProps(), | ||
| contractVersion: MESHERY_EXTENSION_CONTRACT_VERSION | ||
| }); | ||
|
|
||
| expect(report.contractVersionMismatch).toBeNull(); | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(true); | ||
| }); | ||
|
|
||
| it('treats a host that advertises no version as the supported legacy case', () => { | ||
| // Every host deployed before this handshake existed. They predate the field | ||
| // rather than disagreeing about it, so flagging them would make the report | ||
| // fire everywhere at once and teach maintainers to ignore it. | ||
| const report = reportInjectedCapabilities(satisfyingInjectProps()); | ||
|
|
||
| expect(report.contractVersionMismatch).toBeNull(); | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(true); | ||
| expect(describeInjectedCapabilityReport(report)).toBeNull(); | ||
| }); | ||
|
|
||
| it('fails the handshake loudly when the advertised version differs', () => { | ||
| const report = reportInjectedCapabilities({ | ||
| ...satisfyingInjectProps(), | ||
| contractVersion: MESHERY_EXTENSION_CONTRACT_VERSION + 1 | ||
| }); | ||
|
|
||
| expect(report.contractVersionMismatch).toEqual({ | ||
| host: MESHERY_EXTENSION_CONTRACT_VERSION + 1, | ||
| extension: MESHERY_EXTENSION_CONTRACT_VERSION | ||
| }); | ||
| // Every capability is present, so nothing else in the report would catch this. | ||
| expect(report.missing).toEqual([]); | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(false); | ||
|
|
||
| const message = describeInjectedCapabilityReport(report); | ||
| expect(message).toContain(`v${MESHERY_EXTENSION_CONTRACT_VERSION + 1}`); | ||
| expect(message).toContain(`v${MESHERY_EXTENSION_CONTRACT_VERSION}`); | ||
| }); | ||
|
|
||
| it('treats a non-numeric advertised version as a mismatch rather than trusting it', () => { | ||
| // `injectProps` is untrusted input; a version we cannot compare is not a | ||
| // version we may assume matches. | ||
| const report = reportInjectedCapabilities({ | ||
| ...satisfyingInjectProps(), | ||
| contractVersion: `${MESHERY_EXTENSION_CONTRACT_VERSION}` | ||
| }); | ||
|
|
||
| expect(report.contractVersionMismatch).toEqual({ | ||
| host: `${MESHERY_EXTENSION_CONTRACT_VERSION}`, | ||
| extension: MESHERY_EXTENSION_CONTRACT_VERSION | ||
| }); | ||
| expect(isInjectedCapabilityReportSatisfied(report)).toBe(false); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,3 +39,5 @@ export { | |
| } from './utils'; | ||
|
|
||
| export * from './eventBus'; | ||
|
|
||
| export * from './mesheryExtensionContract'; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The type-only contract gets 175 lines of tests; the actual runtime bug fix in this PR gets none.
Per the repo's own standard that tests ship with the change: this suite is genuinely good, but it exercises a module that is almost entirely compile-time. The
DateTimePickerchange is the part of this PR that fixes a real, reported, runtime failure - "in Kanvas that took out four unrelated Jest suites on a clean install" - and nothing here would catch its regression, or catch the same bug appearing in a different module.That is not hypothetical: the same bug is live in this PR via
date-fnsinUniversalFilter(see theDateTimePicker.tsxthread). A test asserting the property rather than the component would have caught both.Added
src/__testing__/optionalPeerDependencies.test.ts, which walks every non-test module undersrc/and fails if any of them eagerly imports an optional peer, listing the offending paths. It correctly ignoresawait import(...)and type-only imports, and separately asserts that both peers are still markedoptionalinpackage.jsonso the test's premise cannot silently change.Also added
src/__testing__/date.utils.test.tsfor the replacement date helpers, covering the end-of-month and leap-year clamping that a naive implementation gets wrong.