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

fix(compiler-cli): better detect classes that are indirectly exported #42207

Closed
wants to merge 1 commit into from

Conversation

alxhub
Copy link
Member

@alxhub alxhub commented May 20, 2021

The compiler flag compileNonExportedClasses allows the Angular compiler to
process classes which are not exported at the top level of a source file.
This is often used to allow for AOT compilation of test classes inside
it() test blocks, for example.

Previously, the compiler would identify exported classes by looking for an
export modifier on the class declaration itself. This works for the
trivial case, but fails for indirectly exported classes:

// Component is declared unexported.
@Component({...})
class FooCmp {...}

// Indirect export of FooCmp
export {FooCmp};

This is not an immediate problem for most application builds, since the
default value for compileNonExportedClasses is true and therefore such
classes get compiled regardless.

However, in the Angular Language Service now, compileNonExportedClasses is
forcibly overridden to false. That's because the tsconfig used by the IDE
and Language Service is often far broader than the application build's
configuration, and pulls in test files that can contain unexported classes
not designed with AOT compilation in mind.

Therefore, the Language Service has trouble working with such structures.

In this commit, the ReflectionHost gains a new API for detecting whether a
class is exported. The implementation of this method now not only considers
the export modifier, but also scans the ts.SourceFile for indirect
exports like the example above. This ensures the above case will be
processed directly in the Language Service.

This new operation is cached using an expando symbol on the ts.SourceFile,
ensuring good performance even when scanning large source files with lots of
exports (e.g. a FESM file under ngcc).

Fixes #42184.

@google-cla google-cla bot added the cla: yes label May 20, 2021
@alxhub alxhub requested a review from atscott May 20, 2021 20:48
@zarend zarend added the area: compiler Issues related to `ngc`, Angular's template compiler label May 20, 2021
@ngbot ngbot bot added this to the Backlog milestone May 20, 2021
The compiler flag `compileNonExportedClasses` allows the Angular compiler to
process classes which are not exported at the top level of a source file.
This is often used to allow for AOT compilation of test classes inside
`it()` test blocks, for example.

Previously, the compiler would identify exported classes by looking for an
`export` modifier on the class declaration itself. This works for the
trivial case, but fails for indirectly exported classes:

```typescript
// Component is declared unexported.
@component({...})
class FooCmp {...}

// Indirect export of FooCmp
export {FooCmp};
```

This is not an immediate problem for most application builds, since the
default value for `compileNonExportedClasses` is `true` and therefore such
classes get compiled regardless.

However, in the Angular Language Service now, `compileNonExportedClasses` is
forcibly overridden to `false`. That's because the tsconfig used by the IDE
and Language Service is often far broader than the application build's
configuration, and pulls in test files that can contain unexported classes
not designed with AOT compilation in mind.

Therefore, the Language Service has trouble working with such structures.

In this commit, the `ReflectionHost` gains a new API for detecting whether a
class is exported. The implementation of this method now not only considers
the `export` modifier, but also scans the `ts.SourceFile` for indirect
exports like the example above. This ensures the above case will be
processed directly in the Language Service.

This new operation is cached using an expando symbol on the `ts.SourceFile`,
ensuring good performance even when scanning large source files with lots of
exports (e.g. a FESM file under `ngcc`).

Fixes angular#42184.
@alxhub alxhub force-pushed the ngtsc/export-means-export branch from 90ee23b to c8b1bc6 Compare May 20, 2021 21:29
@atscott atscott added the target: patch This PR is targeted for the next patch release label Jun 1, 2021
@atscott
Copy link
Contributor

atscott commented Jun 1, 2021

presubmit

@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Jun 1, 2021
AndrewKushnir pushed a commit that referenced this pull request Jun 1, 2021
…#42207)

The compiler flag `compileNonExportedClasses` allows the Angular compiler to
process classes which are not exported at the top level of a source file.
This is often used to allow for AOT compilation of test classes inside
`it()` test blocks, for example.

Previously, the compiler would identify exported classes by looking for an
`export` modifier on the class declaration itself. This works for the
trivial case, but fails for indirectly exported classes:

```typescript
// Component is declared unexported.
@component({...})
class FooCmp {...}

// Indirect export of FooCmp
export {FooCmp};
```

This is not an immediate problem for most application builds, since the
default value for `compileNonExportedClasses` is `true` and therefore such
classes get compiled regardless.

However, in the Angular Language Service now, `compileNonExportedClasses` is
forcibly overridden to `false`. That's because the tsconfig used by the IDE
and Language Service is often far broader than the application build's
configuration, and pulls in test files that can contain unexported classes
not designed with AOT compilation in mind.

Therefore, the Language Service has trouble working with such structures.

In this commit, the `ReflectionHost` gains a new API for detecting whether a
class is exported. The implementation of this method now not only considers
the `export` modifier, but also scans the `ts.SourceFile` for indirect
exports like the example above. This ensures the above case will be
processed directly in the Language Service.

This new operation is cached using an expando symbol on the `ts.SourceFile`,
ensuring good performance even when scanning large source files with lots of
exports (e.g. a FESM file under `ngcc`).

Fixes #42184.

PR Close #42207
dimakuba pushed a commit to dimakuba/angular that referenced this pull request Jun 2, 2021
…angular#42207)

The compiler flag `compileNonExportedClasses` allows the Angular compiler to
process classes which are not exported at the top level of a source file.
This is often used to allow for AOT compilation of test classes inside
`it()` test blocks, for example.

Previously, the compiler would identify exported classes by looking for an
`export` modifier on the class declaration itself. This works for the
trivial case, but fails for indirectly exported classes:

```typescript
// Component is declared unexported.
@component({...})
class FooCmp {...}

// Indirect export of FooCmp
export {FooCmp};
```

This is not an immediate problem for most application builds, since the
default value for `compileNonExportedClasses` is `true` and therefore such
classes get compiled regardless.

However, in the Angular Language Service now, `compileNonExportedClasses` is
forcibly overridden to `false`. That's because the tsconfig used by the IDE
and Language Service is often far broader than the application build's
configuration, and pulls in test files that can contain unexported classes
not designed with AOT compilation in mind.

Therefore, the Language Service has trouble working with such structures.

In this commit, the `ReflectionHost` gains a new API for detecting whether a
class is exported. The implementation of this method now not only considers
the `export` modifier, but also scans the `ts.SourceFile` for indirect
exports like the example above. This ensures the above case will be
processed directly in the Language Service.

This new operation is cached using an expando symbol on the `ts.SourceFile`,
ensuring good performance even when scanning large source files with lots of
exports (e.g. a FESM file under `ngcc`).

Fixes angular#42184.

PR Close angular#42207
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modules not exported inline (export class MyModule) are not recognized by language service
3 participants