Skip to content

Commit

Permalink
fix(core): coerce dependency versions to caret ranges (#543)
Browse files Browse the repository at this point in the history
  • Loading branch information
unstubbable authored Nov 1, 2019
1 parent 1477811 commit 915012f
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 14 deletions.
16 changes: 15 additions & 1 deletion docs/guides/writing-a-feature-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@ The `dependencies` map can contain two types of required dependencies:

Feature Service dependencies are declared with their ID as key, and a [semver
version range][semver] as value, e.g.
`{'acme:some-feature-service': '^2.0.0'}`.
`{'acme:some-feature-service': '^2.0.0'}`. Since [Feature Services only
provide the latest minor version for each major
version][providing-a-versioned-api], a [caret range][semver-caret-range]
should be used here. If instead an exact version or a [tilde
range][semver-tilde-range] is used, this will be coerced to a caret range by
the `Feature ServiceRegistry`.

1. With `dependencies.externals` all required external dependencies are
declared. This may include [shared npm
Expand All @@ -67,6 +72,10 @@ log an info message.

Feature Service dependencies are declared with their ID as key, and a [semver
version range][semver] as value, e.g. `{'acme:some-feature-service': '^2.0.0'}`.
Since [Feature Services only provide the latest minor version for each major
version][providing-a-versioned-api], a [caret range][semver-caret-range] should
be used here. If instead an exact version or a [tilde range][semver-tilde-range]
is used, this will be coerced to a caret range by the `Feature ServiceRegistry`.

> **Note:**
> Optional external dependencies (i.e. `optionalDependencies.externals`) are not
Expand Down Expand Up @@ -257,4 +266,9 @@ const myFeatureAppDefinition = {
[react-feature-app]: /docs/guides/writing-a-feature-app#react-feature-app
[sharing-npm-dependencies]: /docs/guides/sharing-npm-dependencies
[semver]: https://semver.org
[semver-caret-range]:
https://docs.npmjs.com/misc/semver#caret-ranges-123-025-004
[semver-tilde-range]: https://docs.npmjs.com/misc/semver#tilde-ranges-123-12-1
[issue-245]: https://github.com/sinnerschrader/feature-hub/issues/245
[providing-a-versioned-api]:
/docs/guides/writing-a-feature-service#providing-a-versioned-api
11 changes: 8 additions & 3 deletions docs/guides/writing-a-feature-service.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ The `dependencies` map can contain two types of required dependencies:
version range][semver] as value, e.g.
`{'acme:other-feature-service': '^2.0.0'}`. Since
[Feature Services only provide the latest minor version for each major version](#providing-a-versioned-api),
a [caret range][semver-caret-range] should be used here.
a [caret range][semver-caret-range] should be used here. If instead an exact
version or a [tilde range][semver-tilde-range] is used, this will be coerced
to a caret range by the `Feature ServiceRegistry`.

1. With `dependencies.externals` all required external dependencies are
declared. This may include [shared npm
Expand Down Expand Up @@ -240,8 +242,10 @@ const myFeatureServiceDefinition = {
> A Feature Service needs to provide only one implementation per major version,
> since minor versions only add new features, and thus the latest minor version
> also satisfies the consumers of all previous minor versions of the same major
> version. For this reason, consumers should specify [caret
> ranges][semver-caret-range] for their [dependencies](#dependencies).
> version. For this reason, the `FeatureServiceRegistry` coerces the versions
> that consumers define in their [dependencies](#dependencies) to [caret
> ranges][semver-caret-range]. Thus, it is safe to to provide only one
> implementation per major version.
## Managing Consumer-specific State

Expand Down Expand Up @@ -308,6 +312,7 @@ const myFeatureServiceDefinition = {
[semver]: https://semver.org
[semver-caret-range]:
https://docs.npmjs.com/misc/semver#caret-ranges-123-025-004
[semver-tilde-range]: https://docs.npmjs.com/misc/semver#tilde-ranges-123-12-1
[own-feature-service-definitions]:
/docs/guides/writing-a-feature-app#ownfeatureservicedefinitions
[issue-245]: https://github.com/sinnerschrader/feature-hub/issues/245
62 changes: 56 additions & 6 deletions packages/core/src/__tests__/feature-service-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ describe('FeatureServiceRegistry', () => {
it('fails to register a Feature Service due to an unsupported dependency version', () => {
const stateProviderD = {
id: 'd',
dependencies: {featureServices: {a: '~1.0'}},
dependencies: {featureServices: {a: '^2.0.0'}},
create: jest.fn()
};

Expand All @@ -227,15 +227,15 @@ describe('FeatureServiceRegistry', () => {
)
).toThrowError(
new Error(
'The required Feature Service "a" in the unsupported version range "~1.0" could not be bound to consumer "d". The supported versions are ["1.1.0"].'
'The required Feature Service "a" in the unsupported version range "^2.0.0" could not be bound to consumer "d". The supported versions are ["1.1.0"].'
)
);
});

it('does not fail to register a Feature Service due to an unsupported optional dependency version', () => {
const stateProviderD = {
id: 'd',
optionalDependencies: {featureServices: {a: '~1.0'}},
optionalDependencies: {featureServices: {a: '^2.0.0'}},
create: jest.fn(() => ({}))
};

Expand All @@ -251,7 +251,7 @@ describe('FeatureServiceRegistry', () => {
'The Feature Service "a" has been successfully registered by registrant "test".'
],
[
'The optional Feature Service "a" in the unsupported version range "~1.0" could not be bound to consumer "d". The supported versions are ["1.1.0"].'
'The optional Feature Service "a" in the unsupported version range "^2.0.0" could not be bound to consumer "d". The supported versions are ["1.1.0"].'
],
[
'The Feature Service "d" has been successfully registered by registrant "test".'
Expand Down Expand Up @@ -465,7 +465,7 @@ describe('FeatureServiceRegistry', () => {
});
});

describe('for a Feature Service consumer with dependencies', () => {
describe('for a Feature Service consumer with caret range dependencies', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry({logger});

Expand All @@ -478,7 +478,57 @@ describe('FeatureServiceRegistry', () => {

expect(
featureServiceRegistry.bindFeatureServices(
{dependencies: {featureServices: {a: '1.1.0'}}},
{dependencies: {featureServices: {a: '^1.0.0'}}},
'foo'
)
).toEqual({
featureServices: {a: featureServiceA},
unbind: expect.any(Function)
});

expect(binderA.mock.calls).toEqual([['foo']]);
});
});

describe('for a Feature Service consumer with tilde range dependencies', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry({logger});

featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);

expect(binderA.mock.calls).toEqual([]);

expect(
featureServiceRegistry.bindFeatureServices(
{dependencies: {featureServices: {a: '~1.0.0'}}},
'foo'
)
).toEqual({
featureServices: {a: featureServiceA},
unbind: expect.any(Function)
});

expect(binderA.mock.calls).toEqual([['foo']]);
});
});

describe('for a Feature Service consumer with exact dependencies', () => {
it('creates a bindings object with Feature Services', () => {
featureServiceRegistry = new FeatureServiceRegistry({logger});

featureServiceRegistry.registerFeatureServices(
[providerDefinitionA],
'test'
);

expect(binderA.mock.calls).toEqual([]);

expect(
featureServiceRegistry.bindFeatureServices(
{dependencies: {featureServices: {a: '1.0.0'}}},
'foo'
)
).toEqual({
Expand Down
11 changes: 7 additions & 4 deletions packages/core/src/feature-service-registry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {satisfies, valid} from 'semver';
import {coerce, satisfies, valid} from 'semver';
import {ExternalsValidator, RequiredExternals} from './externals-validator';
import * as Messages from './internal/feature-service-registry-messages';
import {
Expand Down Expand Up @@ -342,7 +342,9 @@ export class FeatureServiceRegistry {
versionRange: string | undefined,
{optional}: {optional: boolean}
): FeatureServiceBinding<unknown> | undefined {
if (!versionRange) {
const coercedVersion = versionRange && coerce(versionRange);

if (!coercedVersion) {
const message = Messages.featureServiceDependencyVersionInvalid(
optional,
providerId,
Expand All @@ -358,6 +360,7 @@ export class FeatureServiceRegistry {
throw new Error(message);
}

const caretRange = `^${coercedVersion.version}`;
const sharedFeatureService = this.sharedFeatureServices.get(providerId);

if (!sharedFeatureService) {
Expand All @@ -379,7 +382,7 @@ export class FeatureServiceRegistry {
const supportedVersions = Object.keys(sharedFeatureService);

const version = supportedVersions.find(supportedVersion =>
satisfies(supportedVersion, versionRange)
satisfies(supportedVersion, caretRange)
);

const bindFeatureService = version && sharedFeatureService[version];
Expand All @@ -389,7 +392,7 @@ export class FeatureServiceRegistry {
optional,
providerId,
consumerId,
versionRange,
caretRange,
supportedVersions
);

Expand Down

0 comments on commit 915012f

Please sign in to comment.