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

Property naming idea(s) #16

Closed
kylecordes opened this issue Apr 11, 2017 · 5 comments
Closed

Property naming idea(s) #16

kylecordes opened this issue Apr 11, 2017 · 5 comments

Comments

@kylecordes
Copy link
Collaborator

This is an issue to kick around ideas for property naming (Closure renaming...) of properties in objects exposed by Components to templates.

I'm not a fan of a mechanism to further automate forcing quoted property identifiers that don't get renamed - because:

  • That gives away optimization
  • It kind of breaks JavaScript (which TS is an extension of), which makes a.x == a['x']
  • Changing my Component and Service code to use more quoted property names is ugly, and thus creates resistance to the ABC future.

Rather, I'd like Angular + tsickle + CC to meet the following contract:

IF, as a developer, I:

  • use typed data structures for any data exposed by a component to a template
  • turn TypeScript type safety (-ish) as high as it will code, with the new string option
  • use consistently, idiomatically unquoted field identifiers

THEN,

  • Angular + tsickle will generate enough detailed typing that CC is fully aware of these (record?) types
  • Angular compiler will generate non-quoted-style access in compiled templates
  • ... so that CC can see types across the whole code base (template, components, services, etc.)
  • ... and can perform aggressive renaming without breaking anything

Is this feasible? Desirable?

cc @alexeagle @thelgevold

@alexeagle
Copy link
Owner

The thing we could automate is that tsickle sees

declare interface MyJsonData {
  x: string;
}

let data: MyJsonData;
data.x;

it could automatically convert to data['x'] because it knows MyJsonData fields should not be renamed. That's what Mohamed suggested on microsoft/TypeScript#14267

@kylecordes
Copy link
Collaborator Author

@alexeagle I read that also, and it seemed like a solution for the problem how to talk about data from the outside world. When setting up interfaces that describe data coming from an external source - JSON API or whatever - a developer would use that extra word of syntax, then Tsickle generates output such that Closure knows to preserve field names.

But...

I'd prefer, if possible to not treat Angular templates as external code that needs such treatment. Rather, compile templates + app code in such a way that Closure can do its whole-program-optimization magic, shortening field names, etc.

With this further elaboration - does it seem feasible? Or is the best we can aim for, to treat Angular templates as "outside", and automatically flag data headed there to not rename fields?

@thelgevold
Copy link
Contributor

@kylecordes My experience so far is that template references already work that way. I have only had to quote properties when referring to properties in http responses.

Example here: https://github.com/thelgevold/closure-compiler-angular-bundling-old/tree/migrate-demo/src/components/tree-view

In the Treeview I define properties that are referenced from html and the component class. It generally seems to work as intended.

Are you seeing cases where this breaks?

@kylecordes
Copy link
Collaborator Author

Here is a case that briefly surprised me - FormBuilder.group. Here is an example change I made to a snip of code for Closure compatibility.

screen shot 2017-04-13 at 9 53 45 am

@alexeagle
Copy link
Owner

@chuckjaz had some discussion with TypeScript team about this a couple weeks ago. I think we should plan sometime maybe in Q1 to have a serious design discussion about safer type-checking for property renaming. In the meantime let's move discussion to microsoft/TypeScript#14267

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

No branches or pull requests

3 participants