This repository has been archived by the owner on Sep 9, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 9
WIP: TypeScript #5
Closed
Closed
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 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,11 @@ | ||
| export const name: string; | ||
|
|
||
| export default class Modifier { | ||
| element: HTMLElement | SVGElement; | ||
| didInsertElement(positionalArgs?: Array<any>, namedArgs?: { [key: string]: any; }): void; | ||
| didRecieveArguments(positionalArgs?: Array<any>, namedArgs?: { [key: string]: any; }): void; | ||
| didUpdateArguments(positionalArgs?: Array<any>, namedArgs?: { [key: string]: any; }): void; | ||
| willDestroyElement(positionalArgs?: Array<any>, namedArgs?: { [key: string]: any; }): void; | ||
|
|
||
| static modifier(klass: typeof Modifier): typeof Modifier; | ||
| } | ||
This file contains 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
This file contains 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,16 @@ | ||
| export default config; | ||
|
|
||
| /** | ||
| * Type declarations for | ||
| * import config from './config/environment' | ||
| * | ||
| * For now these need to be managed by the developer | ||
| * since different ember addons can materialize new entries. | ||
| */ | ||
| declare const config: { | ||
| environment: any; | ||
| modulePrefix: string; | ||
| podModulePrefix: string; | ||
| locationType: string; | ||
| rootURL: string; | ||
| }; |
This file contains 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
246 changes: 246 additions & 0 deletions
246
tests/integration/modifier-managers/oo-modifiers-native-ts-test.ts
This file contains 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,246 @@ | ||
| import { module, test } from 'qunit'; | ||
| import { setupRenderingTest } from 'ember-qunit'; | ||
| import { render, settled } from '@ember/test-helpers'; | ||
| import Service from '@ember/service'; | ||
| import { inject as service } from '@ember-decorators/service'; | ||
| import hbs from 'htmlbars-inline-precompile'; | ||
| import Modifier from 'ember-oo-modifiers'; | ||
|
|
||
| module('Integration | Modifier Manager | oo modifier (native) (TS)', function(hooks) { | ||
| setupRenderingTest(hooks); | ||
|
|
||
| hooks.beforeEach(function() { | ||
| this.registerModifier = (name, modifier) => { | ||
| this.owner.register(`modifier:${name}`, modifier); | ||
| }; | ||
| this.registerModifierClass = (name: string, ModifierClass: typeof Modifier) => { | ||
| this.registerModifier(name, Modifier.modifier(ModifierClass)); | ||
| }; | ||
| }); | ||
|
|
||
| module('didInsertElement', function() { | ||
| test('it has DOM element on this.element', async function(assert) { | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didInsertElement() { assert.equal(this.element.tagName, 'H1'); } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird}}>Hello</h1>`); | ||
| }); | ||
|
|
||
| test('positional arguments are passed', async function(assert) { | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didInsertElement([a, b]: [string, string]) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird "1" "2"}}>Hey</h1>`); | ||
| }); | ||
|
|
||
| test('named arguments are passed', async function(assert) { | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didInsertElement(_: any, { a, b }: { [key: string]: string }) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird a="1" b="2"}}>Hey</h1>`); | ||
| }); | ||
| }); | ||
|
|
||
| module('didRecieveArguments', function() { | ||
| test('it has DOM element on this.element', async function(assert) { | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didReceiveArguments() { assert.equal(this.element.tagName, 'H1'); } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird}}>Hello</h1>`); | ||
| }); | ||
|
|
||
| test('positional arguments are passed', async function(assert) { | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didReceiveArguments([a, b]: [string, string]) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird "1" "2"}}>Hey</h1>`); | ||
| }); | ||
|
|
||
| test('named arguments are passed', async function(assert) { | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didReceiveArguments(_: any, { a, b }: { [key: string]: string }) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird a="1" b="2"}}>Hey</h1>`); | ||
| }); | ||
| }); | ||
|
|
||
| module('didUpdateArguments', function() { | ||
| test('it has DOM element on this.element', async function(assert) { | ||
| this.value = 0; | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didUpdateArguments() { assert.equal(this.element.tagName, 'H1'); } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird this.value}}>Hello</h1>`); | ||
| this.set('value', 1); | ||
| }); | ||
|
|
||
| test('positional arguments are passed', async function(assert) { | ||
| this.value = 0; | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didUpdateArguments([, a, b]: [any, string, string]) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird this.value "1" "2"}}>Hey</h1>`); | ||
| this.set('value', 1); | ||
| }); | ||
|
|
||
| test('named arguments are passed', async function(assert) { | ||
| this.value = 0; | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didUpdateArguments(_: any, { a, b }: { [key: string]: string }) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird this.value a="1" b="2"}}>Hey</h1>`); | ||
| this.set('value', 1); | ||
| }); | ||
| }); | ||
|
|
||
| module('willDestroyElement', function() { | ||
| test('it has DOM element on this.element', async function(assert) { | ||
| this.shouldRender = true; | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| willDestroyElement() { assert.equal(this.element.tagName, 'H1'); } | ||
| } | ||
| ); | ||
| await render(hbs` | ||
| {{#if this.shouldRender}} | ||
| <h1 {{songbird}}>Hello</h1> | ||
| {{/if}} | ||
| `); | ||
| this.set('shouldRender', false); | ||
| }); | ||
|
|
||
| test('positional arguments are passed', async function(assert) { | ||
| this.shouldRender = true; | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| willDestroyElement([a, b]: [string, string]) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs` | ||
| {{#if this.shouldRender}} | ||
| <h1 {{songbird "1" "2"}}>Hey</h1> | ||
| {{/if}} | ||
| `); | ||
| this.set('shouldRender', false); | ||
| }); | ||
|
|
||
| test('named arguments are passed', async function(assert) { | ||
| this.shouldRender = true; | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| willDestroyElement(_: any, { a, b }: { [key: string]: string }) { | ||
| assert.equal(a, '1'); | ||
| assert.equal(b, '2'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs` | ||
| {{#if this.shouldRender}} | ||
| <h1 {{songbird a="1" b="2"}}>Hey</h1> | ||
| {{/if}} | ||
| `); | ||
| this.set('shouldRender', false); | ||
| }); | ||
| }); | ||
|
|
||
| test('has correct lifecycle hooks ordering', async function(assert) { | ||
| let callstack = []; | ||
| this.value = 0; | ||
| this.shouldRender = true; | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| didInsertElement() { callstack.push('didInsertElement'); } | ||
| didReceiveArguments() { callstack.push('didReceiveArguments'); } | ||
| didUpdateArguments() { callstack.push('didUpdateArguments'); } | ||
| willDestroyElement() { callstack.push('willDestroyElement'); } | ||
| } | ||
| ); | ||
| await render(hbs` | ||
| {{#if this.shouldRender}} | ||
| <h1 {{songbird this.value}}>Hey</h1> | ||
| {{/if}} | ||
| `); | ||
| this.set('value', 1); | ||
| await settled(); | ||
| this.set('shouldRender', false); | ||
| await settled(); | ||
| assert.deepEqual(callstack, [ | ||
| 'didInsertElement', | ||
| 'didReceiveArguments', | ||
| 'didReceiveArguments', | ||
| 'didUpdateArguments', | ||
| 'willDestroyElement' | ||
| ]); | ||
| }); | ||
|
|
||
| test('can participate in ember dependency injection', async function(assert) { | ||
| this.owner.register( | ||
| 'service:test-service', | ||
| class TestService extends Service { | ||
| value = 'test-service-value' | ||
| } | ||
| ); | ||
| this.registerModifierClass( | ||
| 'songbird', | ||
| class SongbirdModifier extends Modifier { | ||
| @service testService | ||
| didInsertElement() { | ||
| assert.equal(this.testService.value, 'test-service-value'); | ||
| } | ||
| } | ||
| ); | ||
| await render(hbs`<h1 {{songbird}}>Hello</h1>`); | ||
| }); | ||
| }); |
This file contains 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,55 @@ | ||
| { | ||
| "compilerOptions": { | ||
| "target": "es2017", | ||
| "allowJs": true, | ||
| "moduleResolution": "node", | ||
| "allowSyntheticDefaultImports": true, | ||
| "noImplicitAny": true, | ||
| "noImplicitThis": true, | ||
| "alwaysStrict": true, | ||
| "strictNullChecks": true, | ||
| "strictPropertyInitialization": true, | ||
| "noFallthroughCasesInSwitch": true, | ||
| "noUnusedLocals": true, | ||
| "noUnusedParameters": true, | ||
| "noImplicitReturns": true, | ||
| "noEmitOnError": false, | ||
| "noEmit": true, | ||
| "inlineSourceMap": true, | ||
| "inlineSources": true, | ||
| "baseUrl": ".", | ||
| "module": "es6", | ||
| "paths": { | ||
| "dummy/tests/*": [ | ||
| "tests/*" | ||
| ], | ||
| "dummy/*": [ | ||
| "tests/dummy/app/*", | ||
| "app/*" | ||
| ], | ||
| "ember-oo-modifiers": [ | ||
| "addon" | ||
| ], | ||
| "ember-oo-modifiers/*": [ | ||
| "addon/*" | ||
| ], | ||
| "ember-oo-modifiers/test-support": [ | ||
| "addon-test-support" | ||
| ], | ||
| "ember-oo-modifiers/test-support/*": [ | ||
| "addon-test-support/*" | ||
| ], | ||
| "*": [ | ||
| "types/*" | ||
| ] | ||
| } | ||
| }, | ||
| "include": [ | ||
| "app/**/*", | ||
| "addon/**/*", | ||
| "tests/**/*", | ||
| "types/**/*", | ||
| "test-support/**/*", | ||
| "addon-test-support/**/*" | ||
| ] | ||
| } |
This file contains 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 @@ | ||
|
|
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.
Sorry, there have been some changes on master. The typing is now:
There is also the classic API too, but I have no idea what's the best practice for typing things that inherit from
Ember.Object. Maybe @chriskrycho will know.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.
I’ll take a look and make a recommendation that way tomorrow!
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 types should be a single
index.d.tsfile with the following definitions:There are a number of important things to note here.
First, I've replaced
anywithunknown. This means that TS consumers will need to do a runtime check to narrow the types from "anything" to "the actual type". (There are some features landing in TS 3.7 which will make Ember's debug-onlyassertwork perfectly for this, so keep your eyes open for that!) This is roughly what you have to do in a well-behaved modifier anyway, so it's not an extra burden, just helpful guidance from the type system.Second, this sets up the
Argsdefinition with a generic. This is so that the user can extend it with their own args in a way analogous to how the types for@glimmer/componentwork. Users of the modern, class-based API can write something like this, and it'll just work (with the caveat that they'll have to do exactly those runtime checks required by usingunknownabove):However, they'll also be able to do this, supplying a narrower type:
Importantly, that will not type-check if the interface passed into the generic is not compatible with the required
ModifierArgstype. (Note here that TS users will likely want to use debugasserton those arg types in theconstructoruntil we have a solution in place for type-checking template invocation, which is unlikely to happen this calendar year.)Third, that second point is only applicable to native classes. We don't have a way to make TypeScript's type system support that kind of use of generics with classic classes in a straightforward way, and we're not going to invest the time to try to work around it since the community is actively moving toward native classes—and classic classes have basically been non-starters for TS consumers for a long time anyway, for precisely this reason. Having the API in place as we do here is good, and this level of support matches the second-class
Fourth, and this is the most important, we don't have a way to test this at present. As such, please request a review from me or one of the other Typed Ember contributors on changes to this file. I'll be putting together guidance over the next few months about how to use dtslint or tsd (or the two in combination) to provide tests for the exported types, which should help with this!
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.
One addendum: that
type Owner = unknowncan be replaced with an import if we have a public API import forOwner; I just don't know where that exists if so.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.
Thanks @chriskrycho
I'll update this branch with your recommendations.
I will also add your examples to the README.