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

Allow evaluation of a detached buffer in a specified context (angular2 templates) #5470

Closed
alexeagle opened this issue Oct 30, 2015 · 13 comments
Assignees
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@alexeagle
Copy link
Contributor

Angular/Typescript teams discussed this in person.

Angular 2 includes a template parser, and an expression language which may be used in places in the template.
We would like to translate the template to a TypeScript buffer, and pass the buffer to the language services for things like producing semantic errors, requesting intellisense, and the other usual editor features.
However, the template has a backing class (actually a virtual class per @tbosch ) where the fields may be referenced by template expressions. In our canonical example,

@Component({selector: 'greet', template: 'Hello {{name}}!'})
 class Greet {
   name: string;
   constructor() {
     this.name = 'World';
   }
 }

The template could be represented in TypeScript with the expression

`Hello ${this.name}`

In order to get intellisense or errors for that expression, we need to evaluate it in a place where this.name is defined. The obvious way to do this is for us to pass a "detached buffer", meaning a range of the SourceFile which lives outside the file content. It could look like (just a wild stab at possible syntax):

__template_eval(): string { return `Hello ${this.name}`; }

as if this code existed inside the Greet class.

Also we would expect any ranges in the result to give the pos and width locations relative to the buffer we passed, not the Greet class.

@alexeagle alexeagle changed the title Allow evaluation of a buffer in a specified context (angular2 templates) Allow evaluation of a detached buffer in a specified context (angular2 templates) Oct 30, 2015
@weswigham
Copy link
Member

Potentially duplicates #4169

@DanielRosenwasser
Copy link
Member

Sounds more like #5151.

@alexeagle
Copy link
Contributor Author

Certainly related to those, and it would be better for us to solve this in a generalizable way.
That said, I would prefer to keep working with a specific goal in mind in this issue.

@mhegazy mhegazy added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Nov 2, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Nov 2, 2015

@billti has been looking into this, we need to come up with an extensibility model for the language service that allows for non-ts file services to be provided, be it completion, goto def, find all refs, etc..

@billti
Copy link
Member

billti commented Nov 2, 2015

Re the above example: How does the template engine know to rewrite {{name}} as this.name? I assume it wouldn't rewrite {{Date}} as this.Date? (It's almost as though it is wrapped in a JavaScript with(this){...} block, however TypeScript doesn't support with).

It seems as though the template engine already needs to understand some of the scoping/context from the language service, to generate the code to give back to the language service.

@DanielRosenwasser
Copy link
Member

(as a minor clarification, TypeScript "supports" with, but all entities are typed as any within a with context)

@billti
Copy link
Member

billti commented Nov 2, 2015

"Supports" is generous. It emits valid code, but any usage gives you a build error of file1.ts(3,7): error TS2410: All symbols within a 'with' block will be resolved to 'any'.

If you don't mind errors in your project, then yes, it's supported. 😉

@tbosch
Copy link

tbosch commented Nov 3, 2015

So {{Date}} wouldn't work in Angular either. Adding this. before the
expression should be safe...

On Mon, Nov 2, 2015 at 3:43 PM Bill Ticehurst notifications@github.com
wrote:

"Supports" is generous. It emits valid code, but any usage gives you a
build error of file1.ts(3,7): error TS2410: All symbols within a 'with'
block will be resolved to 'any'.

If you don't mind errors in your project, then yes, it's supported. [image:
😉]


Reply to this email directly or view it on GitHub
#5470 (comment)
.

@billti
Copy link
Member

billti commented Nov 3, 2015

Ah, interesting. So in the simple case does it assume all identifiers are members on the instance?

One area where I see this isn't safe is if I'm within a template which introduces a local, such as *ng-for='#name of items'. Now {{name}} within a nested tag refers to the introduced local, even if there is a name property on the class instance for the component.

So it seems the expressions would need to be aware of surrounding tags and any locals they introduce, before they can assume <id> => this.<id>. Is that correct?

@billti
Copy link
Member

billti commented Nov 3, 2015

Actually, on re-reading the blog, can sibling elements introduce names too? e.g. player in the below:

<video-player #player></video-player>
<button (click)="player.pause()">Pause</button>

EDIT: Adding this link as a useful reference: https://angular.io/docs/ts/latest/guide/template-syntax.html#template-expressions . Note syntax changes to "regular" JavaScript as well as "expression context".

To circle back to the original point however: So it would seem the template engine doesn't need context from the underlying component code to understand its references and generate its code. Any identifiers it doesn't recognize (e.g. local names, $event, etc.), it will assume should be instance members (i.e. this.<id> references). Does this sound about right?

@mhegazy
Copy link
Contributor

mhegazy commented May 23, 2017

@alexeagle with #12231 in place, do not think we need this issue any longer. can you please confirm?

@chuckjaz
Copy link
Contributor

This is no longer necessary.

@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

thanks! closing.

@mhegazy mhegazy closed this as completed May 24, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants