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

Start removing implicit any errors #15773

Merged
merged 11 commits into from
Nov 1, 2017
Merged

Conversation

smfoote
Copy link
Contributor

@smfoote smfoote commented Oct 26, 2017

Thusfar:

component-managers/abstract.ts (2 errors remain)
component-managers/curly.ts (some errors remain)
component-managers/mount.ts
utils/curly-component-state-bucket.ts

@@ -0,0 +1,17 @@
interface Container {
Copy link
Contributor

Choose a reason for hiding this comment

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

I stubbed out a more limited interface for this in ember-glimmer we should unify

return vm.getSelf().get('ariaRole');
}

class CurlyComponentLayoutCompiler {
static id: string;
public template: any;

constructor(template) {
constructor(template: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I did make a type for template

// tslint:disable-next-line:no-shadowed-variable
let { tagName } = vm.dynamicScope().view;

return PrimitiveReference.create(tagName === '' ? null : tagName || 'div');
}

function ariaRole(vm) {
function ariaRole(vm: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

there is a type exported for this, I think it is called PublicVM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works for the ariaRole function, but not tagName: Property 'view' does not exist on type 'DynamicScope'

}

get(key) {
get(key: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

string

}
return env.getCompiledBlock(CurlyComponentLayoutCompiler, template);
}

templateFor(component, env) {
templateFor(component: ComponentStateBucket, env: Environment): any {
let Template = get(component, 'layout');
Copy link
Contributor

Choose a reason for hiding this comment

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

we have a type for what templates return look at the template .d.ts I updated

export const HashLocation: any;
export const HistoryLocation: any;
export const AutoLocation: any;

Copy link
Contributor

Choose a reason for hiding this comment

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

lets at least type the signature of the functions

Steven Footea added 5 commits October 27, 2017 14:33
Thusfar:

component-managers/abstract.ts (2 errors remain)
component-managers/curly.ts (some errors remain)
component-managers/mount.ts
utils/curly-component-state-bucket.ts
There are a few uses of `any` still, and a few errors I'm still trying
to figure out.
@smfoote smfoote changed the base branch from typed-ember-glimmer to master October 27, 2017 20:45
@smfoote
Copy link
Contributor Author

smfoote commented Oct 31, 2017

Despite the green check, I don't think this is ready yet. There are several TypeScript errors still.

@rwjblue
Copy link
Member

rwjblue commented Nov 1, 2017

@smfoote - Sounds good, let us know when you think it is ready for another round of review and possible merging.

@smfoote
Copy link
Contributor Author

smfoote commented Nov 1, 2017

Since the checks are passing, we might merge this one as is then fix the remaining TypeScript errors in a future PR. I'll leave that up to @krisselden

@krisselden krisselden merged commit 41899c6 into emberjs:master Nov 1, 2017
@smfoote smfoote deleted the implicit-any branch November 1, 2017 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants