Skip to content
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

Add type definitions #119

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@
/.node_modules.ember-try/
/bower.json.ember-try
/package.json.ember-try

# don't bother linting types and type tests
types
type-tests
23 changes: 23 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,26 @@ jobs:
run: yarn install --frozen-lockfile
- name: Run Tests
run: node_modules/.bin/ember try:one ${{ matrix.scenario }} --skip-cleanup

types:
name: type tests

runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
ts-version:
- 4.4
- 4.5
- 4.6
- next

steps:
- uses: actions/checkout@v3
- run: yarn install --frozen-lockfile --non-interactive
- run: yarn add -D typescript@${{ matrix.ts-version }}
- run: ./node_modules/.bin/tsc --project type-tests
- run: ./node_modules/.bin/tsc --project type-tests/with-registry

continue-on-error: ${{ matrix.ts-version == 'next' }}
3 changes: 3 additions & 0 deletions .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,6 @@
/.node_modules.ember-try/
/bower.json.ember-try
/package.json.ember-try

# non-published TypeScript infrastructure
type-tests
62 changes: 62 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,68 @@ moduleForComponent('my-component', 'Integration | Component | my component', {

Note: for Ember before 2.3.0, you'll need to use [ember-getowner-polyfill](https://github.com/rwjblue/ember-getowner-polyfill).

### TypeScript

The library ships with full support for TypeScript usage with the service, helper, and test helpers. The API described above works as expected, with one additional nicety and one caveat.

**Nicety:** the library provides you the ability to define statically your known feature flags by using a *registry* (as you may be familiar with from the registries for Ember's services, Ember Data models, etc.). If you define your keys (in kebab case!) in a registry like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

woohoo! this is awesome!


```ts
// types/index.d.ts, with other types defined for your app


declare module 'ember-feature-flags/services/features' {
export interface FeaturesRegistry {
'feature-a': boolean;
'feature-b': boolean;
}
}
```

Then in your app code, you will get type checking: TS will require you to use one of those keys (or a camel case variant of it), and reject unknown keys.

```ts
import Component from '@glimmer/component';
import { service } from '@ember/service';
import type Features from 'ember-feature-flags/services/features';

export default class Example extends Component {
@service declare features: Features;

get shouldDoSomething() {
return this.features.isEnabled('feature-a'); // ✅
}

get whoops() {
return this.features.isEnabled('not-a-real-feature'); // ❌
}
}
```

This applies to all the values. If you do *not* add any keys to the `FeatureFlags` interface, the types will fall back to simply allowing any string and returning a boolean value.
Copy link
Contributor

Choose a reason for hiding this comment

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

seem it should be FeaturesRegistry instead of FeatureFlags?


**Caveat:** The runtime uses Ember's `unknownProperty` proxy handling to allow direct access on the service itself with the `get` helper. This allows you to access the features directly in a template:

```hbs
{{#if this.features.featureA}}
{{! ... }}
{{/if}}
```

For [Glint](https://github.com/typed-ember/glint), this is impossible to support in a way which would not *also* suggest that you could write `this.features.featureA` in your TypeScript code. Doing that will always return `undefined` until we are able to update the library to use native proxies instead of Ember's `unknownProperty()` method. The `feature-flag` helper does *not* have this restriction, so you should prefer that instead. If you still want to use the service directly instead of using the helper, you can use `get`:

```hbs
{{#if (get this.features 'featureA')}}
{{! ... }}
{{/if}}
```

This will *not* provide autocomplete or type safety, but will work.

#### Stability

This library provides type definitions and follows the current draft of the [Semantic Versioning for TypeScript Types](https://www.semver-ts.org) specification. The public API is all published types. It currently supports TypeScript 4.4, 4.5, and 4.6.

### Development

#### Installation
Expand Down
13 changes: 12 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
},
"devDependencies": {
"@ember/optional-features": "^1.0.0",
"@types/ember__component": "^4.0.8",
"@types/ember__service": "^4.0.0",
"babel-eslint": "^10.0.3",
"broccoli-asset-rev": "^3.0.0",
"ember-cli": "~3.13.1",
Expand All @@ -38,9 +40,11 @@
"ember-try": "^1.2.1",
"eslint-plugin-ember": "^7.1.0",
"eslint-plugin-node": "^10.0.0",
"expect-type": "^0.13.0",
"lerna-changelog": "^0.8.2",
"loader.js": "^4.7.0",
"qunit-dom": "^0.9.0"
"qunit-dom": "^0.9.0",
"typescript": "^4.6.4"
},
"keywords": [
"ember-addon",
Expand Down Expand Up @@ -76,5 +80,12 @@
"documentation": ":memo: Documentation",
"internal": ":house: Internal"
}
},
"typesVersions": {
"*": {
"*": [
"types/*"
]
}
}
}
46 changes: 46 additions & 0 deletions type-tests/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { expectTypeOf } from "expect-type";
import Helper from "@ember/component/helper";
import type Features from "ember-feature-flags/services/features";
import {
enableFeature,
disableFeature,
} from "ember-feature-flags/test-support";
import FeatureFlag from "ember-feature-flags/helpers/feature-flag";

// side-effect import for `withFeature`
import "ember-feature-flags/test-support/helpers/with-feature";

expectTypeOf(enableFeature).toEqualTypeOf<(name: string) => void>();
expectTypeOf(enableFeature("hello")).toEqualTypeOf<void>();
expectTypeOf(disableFeature).toEqualTypeOf<(name: string) => void>();
expectTypeOf(disableFeature("hello")).toEqualTypeOf<void>();

// globally available because of the side effect import
expectTypeOf(withFeature).toEqualTypeOf<(name: string) => void>();
expectTypeOf(withFeature("hello")).toEqualTypeOf<void>();

// The default for all the methods, if you don't add to the registry, is to
// simply use a string key lookup. See `./with-registry/index.ts` for testing
// when there *is* a registry.
declare let features: Features;
expectTypeOf(features.setup).toEqualTypeOf<
(config: Record<string, boolean>) => void
>();

expectTypeOf(features.isEnabled).toEqualTypeOf<(key: string) => boolean>();
expectTypeOf(features.get).toEqualTypeOf<(key: string) => boolean>();
expectTypeOf(features.enable).toEqualTypeOf<
(key: string, value: boolean) => void
>();
expectTypeOf(features.disable).toEqualTypeOf<
(key: string, value: boolean) => void
>();
expectTypeOf(features.flags).toEqualTypeOf<Array<string>>();

expectTypeOf<FeatureFlag>().toEqualTypeOf<
Helper<{
Args: {
Positional: [string];
};
}>
>();
73 changes: 73 additions & 0 deletions type-tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Pulled from ember-cli-typescript's blueprint
{
"compilerOptions": {
"target": "ES2021",
"module": "ES2020",
"moduleResolution": "node",

// Trying to check Ember apps and addons with `allowJs: true` is a recipe
// for many unresolveable type errors, because with *considerable* extra
// configuration it ends up including many files which are *not* valid and
// cannot be: they *appear* to be resolve-able to TS, but are in fact not in
// valid Node-resolveable locations and may not have TS-ready types. This
// will likely improve over time
"allowJs": false,

// --- TS for SemVer Types compatibility
// Strictness settings -- you should *not* change these: Ember code is not
// guaranteed to type check with these set to looser values.
"strict": true,
"noUncheckedIndexedAccess": true,

// Interop: these are viral and will require anyone downstream of your
// package to *also* set them to true. If you *must* enable them to consume
// an upstream package, you should document that for downstream consumers to
// be aware of.
//
// These *are* safe for apps to enable, since they do not *have* downstream
// consumers; but leaving them off is still preferred when possible, since
// it makes it easier to switch between apps and addons and have the same
// rules for what can be imported and how.
"allowSyntheticDefaultImports": false,
"esModuleInterop": false,

// --- Lint-style rules

// TypeScript also supplies some lint-style checks; nearly all of them are
// better handled by ESLint with the `@typescript-eslint`. This one is more
// like a safety check, though, so we leave it on.
"noPropertyAccessFromIndexSignature": true,

// --- Compilation/integration settings
// Setting `noEmitOnError` here allows ember-cli-typescript to catch errors
// and inject them into Ember CLI's build error reporting, which provides
// nice feedback for when
"noEmitOnError": true,

// We use Babel for emitting runtime code, because it's very important that
// we always and only use the same transpiler for non-stable features, in
// particular decorators. If you were to change this to `true`, it could
// lead to accidentally generating code with `tsc` instead of Babel, and
// could thereby result in broken code at runtime.
"noEmit": true,

// Ember makes heavy use of decorators; TS does not support them at all
// without this flag.
"experimentalDecorators": true,

// Support generation of source maps. Note: you must *also* enable source
// maps in your `ember-cli-babel` config and/or `babel.config.js`.
"declaration": true,
"declarationMap": true,
"inlineSourceMap": true,
"inlineSources": true,

// --- custom paths shenanigans since we don't have `exports` yet ---
"baseUrl": "../types",
"paths": {
"ember-feature-flags": ["."],
"ember-feature-flags/*": ["./*"]
}
},
"include": ["index.ts", "../types/*"]
}
90 changes: 90 additions & 0 deletions type-tests/with-registry/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { expectTypeOf } from "expect-type";
import Features from "ember-feature-flags/services/features";
import Helper from "@ember/component/helper";
import {
enableFeature,
disableFeature,
} from "ember-feature-flags/test-support";
import FeatureFlag from "ember-feature-flags/helpers/feature-flag";

// @ts-expect-error -- we have *not* done the side-effect import in this test,
// so there is no `withFeature` in the global scope!
withFeature("whatever", true);

declare module "ember-feature-flags/services/features" {
export interface FeaturesRegistry {
simple: boolean;
"with-dash": boolean;
}
}

expectTypeOf(enableFeature).parameters.toEqualTypeOf<
["simple" | "withDash" | "with-dash"]
>();
expectTypeOf(disableFeature).parameters.toEqualTypeOf<
["simple" | "withDash" | "with-dash"]
>();

declare let features: Features;

// @ts-expect-error -- no calling it without args at all
features.setup();
// no keys is allowed (though: why would you do that?)
expectTypeOf(features.setup({})).toEqualTypeOf<void>();
// any key actually set up is allowed
expectTypeOf(features.setup({ "with-dash": true })).toEqualTypeOf<void>();
expectTypeOf(features.setup({ simple: false })).toEqualTypeOf<void>();
expectTypeOf(
features.setup({
"with-dash": false,
simple: true,
})
).toEqualTypeOf<void>();
// But random keys are *not* allowed
// @ts-expect-error
features.setup({ otherShenanigans: false });

// Both kebab and camel-case are supported
expectTypeOf(features.isEnabled).parameters.toEqualTypeOf<
["withDash" | "with-dash" | "simple"]
>();
// But random keys are disallowed if using the registry
// @ts-expect-error
features.isEnabled("whatever");

// Both kebab and camel-case are supported
expectTypeOf(features.get).parameters.toEqualTypeOf<
["withDash" | "with-dash" | "simple"]
>();
// But random keys are disallowed if using the registry
// @ts-expect-error
features.get("whatever");

expectTypeOf(features.enable)
.parameter(0)
.toEqualTypeOf<"withDash" | "with-dash" | "simple">();
expectTypeOf(features.enable).parameter(1).toEqualTypeOf<boolean>();
// But random keys are disallowed if using the registry
// @ts-expect-error
features.enable("whatever", true);

expectTypeOf(features.disable)
.parameter(0)
.toEqualTypeOf<"withDash" | "with-dash" | "simple">();
expectTypeOf(features.disable).parameter(1).toEqualTypeOf<boolean>();
// But random keys are disallowed if using the registry
// @ts-expect-error
features.disable("whatever", true);

expectTypeOf(features.flags).toEqualTypeOf<
Array<"simple" | "withDash" | "with-dash">
>();

expectTypeOf<FeatureFlag>().toEqualTypeOf<
Helper<{
Args: {
Positional: ["simple" | "withDash" | "with-dash"];
};
}>
>();
declare let ff: FeatureFlag;
4 changes: 4 additions & 0 deletions type-tests/with-registry/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"extends": "../tsconfig.json",
"include": ["./index.ts", "../../types/*"]
}
11 changes: 11 additions & 0 deletions types/helpers/feature-flag.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import Helper from "@ember/component/helper";
import { Keys } from "ember-feature-flags/services/features";
Copy link
Contributor

@SergeAstapov SergeAstapov May 11, 2022

Choose a reason for hiding this comment

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

out of curiosity, should this be import type?

Copy link
Contributor

Choose a reason for hiding this comment

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

our of curiosity, should this be import type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It could, but since it's in a .d.ts file they're all import type effectively.


interface FeatureFlagSignature {
Args: {
Positional: [Keys];
};
}

/** A helper named `feature-flag` to check features in templates */
export default class FeatureFlag extends Helper<FeatureFlagSignature> {}
Loading