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

feat(ivy): ngtsc template sourcemaps #28055

Closed
wants to merge 17 commits into
base: master
from

Conversation

@petebacondarwin
Copy link
Member

petebacondarwin commented Jan 10, 2019

There is a bug in TypeScript Microsoft/TypeScript#29300 that prevents the templateUrl URL from working correctly. It is fixed in TS 3.3. We have a check to fail gracefully if a
version of TS is running that does not include the fix.

For inline templates, only string literals are supported.

The actual mapping from the generated ivy code to the templates needs some work to improve its accuracy and resolution.

@petebacondarwin petebacondarwin requested review from angular/fw-compiler as code owners Jan 10, 2019

@googlebot googlebot added the cla: yes label Jan 10, 2019

@ngbot ngbot bot added this to the needsTriage milestone Jan 10, 2019

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngtsc-template-sourcemaps branch from d2a34f1 to 23fb85f Jan 10, 2019

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Jan 10, 2019

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Jan 10, 2019

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngtsc-template-sourcemaps branch 2 times, most recently from 62f08cd to 8ed8779 Jan 11, 2019

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Jan 11, 2019

@petebacondarwin petebacondarwin requested review from IgorMinar and angular/docs-infra as code owners Jan 14, 2019

@mhevery mhevery closed this in 81df5dc Feb 13, 2019

mhevery added a commit that referenced this pull request Feb 13, 2019

refactor(compiler): use `options` argument for parsers (#28055)
This commit consolidates the options that can modify the
parsing of text (e.g. HTML, Angular templates, CSS, i18n)
into an AST for further processing into a single `options`
hash.

This makes the code cleaner and more readable, but also
enables us to support further options to parsing without
triggering wide ranging changes to code that should not
be affected by these new options.  Specifically, it will let
us pass information about the placement of a template
that is being parsed in its containing file, which is essential
for accurate SourceMap processing.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

refactor(compiler): remove unnecessary `!` operators from lexer (#28055)
When we added the strict null checks, the lexer had some `!`
operators added to prevent the compilation from failing.

This commit resolves this problem correctly and removes the
hacks.

Also the comment

```
// Note: this is always lowercase!
```

has been removed as it is no longer true.

See #24571

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

feat(compiler): support tokenizing a sub-section of an input string (#…
…28055)

The lexer that does the tokenizing can now process only a part the source
string, by passing a `range` property in the `options` argument. The
locations of the nodes that are tokenized will now take into account the
position of the span in the context of the original source string.

This `range` option is, in turn, exposed from the template parser as well.

Being able to process parts of files helps to enable SourceMap support
when compiling inline component templates.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

feat(compiler): support tokenizing escaped strings (#28055)
In order to support source mapping of templates, we need
to be able to tokenize the template in its original context.
When the template is defined inline as a JavaScript string
in a TS/JS source file, the tokenizer must be able to handle
string escape sequences, such as `\n` and `\"` as they
appear in the original source file.

This commit teaches the lexer how to unescape these
sequences, but only when the `escapedString` option is
set to true.  Otherwise there is no change to the tokenizing
behaviour.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

docs(core): tidy up the description of `resolveComponentResources()` (#…
…28055)

There were a number of typos and some of the sentences did not
read well.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

refactor(core): do not remove `templateUrl` when resolving (#28055)
When we resolve a component `templateUrl` we copy the contents of the
resolved template file into the `template` property.

Previously we would then remove the `templateUrl` to indicate that
the component has been resolved. But this meant that we no longer had
access to the URL of the original template file. This is essential for
diagnostics messages about the template compilation.

Now the existence of the `template` property overrides the existence of
`templateUrl`, which allows us to keep the `templateUrl` property.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

refactor(compiler): wrap the jit evaluation in an injectable class (#…
…28055)

When testing JIT code, it is useful to be able to access the
generated JIT source. Previously this is done by spying on the
global `Function` object, to capture the code when it is being
evaluated. This is problematic because you can only capture
the body of the function, and not the arguments, which messes
up line and column positions for source mapping for instance.

Now the code that generates and then evaluates JIT code is
wrapped in a `JitEvaluator` class, making it possible to provide
a mock implementation that can capture the generated source of
the function passed to `executeFunction(fn: Function, args: any[])`.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

fix(compiler): support `sourceMappingURL` comments that have trailing…
… lines (#28055)

Previously the call to `extractSourceMap()` would only work if the
`//#sourceMappingURL ...` was the last line of the file. This doesn't
work if the code is JIT evaluated as the comment is actually the last
line in the body of a function, wrapped by curly-braces.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

fix(core): use the correct template URL in render3 JIT compilation (#…
…28055)

Previously JIT compiled components did not use the correct URL if
the template was resolved from a `templateUrl`.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

fix(core): use the correct generated URL for JIT compiled components (#…
…28055)

Previously the generated code was being mapped to the `templateUrl`
value.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

test(core): update JIT source mapping tests for ivy (#28055)
There are some differences in how ivy maps template source
compared to View Engine.  In this commit we recreate the View Engine
tests for ivy.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

fix(compiler): markup lexer should not capture quotes in attribute va…
…lue (#28055)

When tokenizing markup (e.g. HTML) element attributes
can have quoted or unquoted values (e.g. `a=b` or `a="b"`).
The `ATTR_VALUE` tokens were capturing the quotes, which
was inconsistent and also affected source-mapping.

Now the tokenizer captures additional `ATTR_QUOTE` tokens,
which the HTML related parsers understand and factor into their
token parsing.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

refactor(compiler): capture `sourceSpan` when converting action bindi…
…ngs to output AST (#28055)

The `convertActionBinding()` now accepts an optional `baseSourceSpan`,
which is the start point of the action expression being converted in the
original source code.  This is used to compute the original position of
the output AST nodes.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

fix(compiler): ensure that event handlers have the correct source spa…
…ns (#28055)

When template bindings are being parsed the event handlers
were receiving a source span that included the whole attribute.

Now they get a span that is focussed on the handler itself.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

feat(ivy): add source mappings to compiled Angular templates (#28055)
During analysis, the `ComponentDecoratorHandler` passes the component
template to the `parseTemplate()` function. Previously, there was little or
no information about the original source file, where the template is found,
passed when calling this function.

Now, we correctly compute the URL of the source of the template, both
for external `templateUrl` and in-line `template` cases. Further in the
in-line template case we compute the character range of the template
in its containing source file; *but only in the case that the template is
a simple string literal*. If the template is actually a dynamic value like
an interpolated string or a function call, then we do not try to add the
originating source file information.

The translator that converts Ivy AST nodes to TypeScript now adds these
template specific source mappings, which account for the file where
the template was found, to the templates to support stepping through the
template creation and update code when debugging an Angular application.

Note that some versions of TypeScript have a bug which means they cannot
support external template source-maps. We check for this via the
`canSourceMapExternalTemplates()` helper function and avoid trying to
add template mappings to external templates if not supported.

PR Close #28055

mhevery added a commit that referenced this pull request Feb 13, 2019

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Feb 14, 2019

build: show no warning for large git repos (angular#28055)
This warning pops up every time you try to run a node debug
session via bazel. It is not important.

PR Close angular#28055

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Feb 14, 2019

refactor(compiler): use `options` argument for parsers (angular#28055)
This commit consolidates the options that can modify the
parsing of text (e.g. HTML, Angular templates, CSS, i18n)
into an AST for further processing into a single `options`
hash.

This makes the code cleaner and more readable, but also
enables us to support further options to parsing without
triggering wide ranging changes to code that should not
be affected by these new options.  Specifically, it will let
us pass information about the placement of a template
that is being parsed in its containing file, which is essential
for accurate SourceMap processing.

PR Close angular#28055

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Feb 14, 2019

refactor(compiler): remove unnecessary `!` operators from lexer (angu…
…lar#28055)

When we added the strict null checks, the lexer had some `!`
operators added to prevent the compilation from failing.

This commit resolves this problem correctly and removes the
hacks.

Also the comment

```
// Note: this is always lowercase!
```

has been removed as it is no longer true.

See angular#24571

PR Close angular#28055

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Feb 14, 2019

docs(core): tidy up the description of `resolveComponentResources()` (a…
…ngular#28055)

There were a number of typos and some of the sentences did not
read well.

PR Close angular#28055

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Feb 14, 2019

docs(core): tidy up the description of `resolveComponentResources()` (a…
…ngular#28055)

There were a number of typos and some of the sentences did not
read well.

PR Close angular#28055

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Mar 4, 2019

refactor(compiler): use `options` argument for parsers (angular#28055)
This commit consolidates the options that can modify the
parsing of text (e.g. HTML, Angular templates, CSS, i18n)
into an AST for further processing into a single `options`
hash.

This makes the code cleaner and more readable, but also
enables us to support further options to parsing without
triggering wide ranging changes to code that should not
be affected by these new options.  Specifically, it will let
us pass information about the placement of a template
that is being parsed in its containing file, which is essential
for accurate SourceMap processing.

PR Close angular#28055

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Mar 4, 2019

refactor(compiler): remove unnecessary `!` operators from lexer (angu…
…lar#28055)

When we added the strict null checks, the lexer had some `!`
operators added to prevent the compilation from failing.

This commit resolves this problem correctly and removes the
hacks.

Also the comment

```
// Note: this is always lowercase!
```

has been removed as it is no longer true.

See angular#24571

PR Close angular#28055

petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Mar 4, 2019

docs(core): tidy up the description of `resolveComponentResources()` (a…
…ngular#28055)

There were a number of typos and some of the sentences did not
read well.

PR Close angular#28055

AndrewKushnir added a commit that referenced this pull request Mar 4, 2019

refactor(compiler): use `options` argument for parsers (#28055) (#28736)
This commit consolidates the options that can modify the
parsing of text (e.g. HTML, Angular templates, CSS, i18n)
into an AST for further processing into a single `options`
hash.

This makes the code cleaner and more readable, but also
enables us to support further options to parsing without
triggering wide ranging changes to code that should not
be affected by these new options.  Specifically, it will let
us pass information about the placement of a template
that is being parsed in its containing file, which is essential
for accurate SourceMap processing.

PR Close #28055

PR Close #28736

AndrewKushnir added a commit that referenced this pull request Mar 4, 2019

refactor(compiler): remove unnecessary `!` operators from lexer (#28055
…) (#28736)

When we added the strict null checks, the lexer had some `!`
operators added to prevent the compilation from failing.

This commit resolves this problem correctly and removes the
hacks.

Also the comment

```
// Note: this is always lowercase!
```

has been removed as it is no longer true.

See #24571

PR Close #28055

PR Close #28736

AndrewKushnir added a commit that referenced this pull request Mar 4, 2019

docs(core): tidy up the description of `resolveComponentResources()` (#…
…28055) (#28736)

There were a number of typos and some of the sentences did not
read well.

PR Close #28055

PR Close #28736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.