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.

Sign up for GitHub

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

Jump to bottom

Handle forward references in same file imports #48898

Closed
wants to merge 3 commits into from
Closed

Handle forward references in same file imports #48898

wants to merge 3 commits into from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Feb 1, 2023 •
edited

When we automatically generate imports within the same file, we may have to create a forwardRef in order for the code to compile. This PR is composed of the following changes:

  1. Adds a isForwardReference field to PotentialImport.
  2. Updates the language service to generate a forwardRef.
  3. Updates the standalone migration to generate a forwardRef.

@crisbeto
refactor(compiler): indicate whether potential import is forward refe…
1a9b0fe 
…rence

In the `PotentialImport` we indicate if it's in the same file by not setting a `moduleSpecifier`, but if that's the case, the imported symbol might need to be wrapped in a `forwardRef` to avoid generating an error. These changes expose this information so the various tools can take advantage of it.
@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: language-service Issues related to Angular's VS Code language service area: compiler Issues related to `ngc`, Angular's template compiler area: migrations Issues related to `ng update` migrations target: minor This PR is targeted for the next minor release labels Feb 1, 2023
@ngbot ngbot bot modified the milestone: Backlog Feb 1, 2023
@crisbeto
fix(language-service): generate forwardRef for same file imports
9f3db17 
Adds some logic that will generate a `forwardRef` if necessary when automatically fixing an import.
@crisbeto
fix(migrations): generate forwardRef for same file imports
cd2505d 
Adds some logic that will generate a `forwardRef` if necessary when adding imports.
@crisbeto crisbeto force-pushed the potential-imports-forward-ref branch from 8ea2c48 to cd2505d Compare February 1, 2023 10:53
@crisbeto crisbeto requested a review from dylhunn February 1, 2023 11:22
@crisbeto crisbeto marked this pull request as ready for review February 1, 2023 11:22
@pullapprove pullapprove bot requested a review from alxhub February 1, 2023 11:23
@crisbeto crisbeto requested review from atscott and removed request for alxhub February 2, 2023 08:00
atscott
atscott approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

Cool 😎

@atscott atscott added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 2, 2023
@crisbeto crisbeto removed the request for review from dylhunn February 2, 2023 18:27
@dylhunn
Copy link
Contributor

dylhunn commented Feb 2, 2023

This PR was merged into the repository by commit ba38178.

@dylhunn dylhunn closed this in 59c0106 Feb 2, 2023
dylhunn pushed a commit that referenced this pull request Feb 2, 2023
@crisbeto @dylhunn
fix(language-service): generate forwardRef for same file imports (#48898
d014503 
)

Adds some logic that will generate a `forwardRef` if necessary when automatically fixing an import.

PR Close #48898
dylhunn pushed a commit that referenced this pull request Feb 2, 2023
@crisbeto @dylhunn
fix(migrations): generate forwardRef for same file imports (#48898)
ba38178 
Adds some logic that will generate a `forwardRef` if necessary when adding imports.

PR Close #48898
crisbeto added a commit to crisbeto/angular that referenced this pull request Feb 7, 2023
@crisbeto
fix(compiler-cli): incorrectly detecting forward refs when symbol alr…
2a5b36f 
…eady exists in file

In angular#48898 the `isForwardRef` flag was added to indicate whether a reference should be wrapped in a `forwardRef`. This logic assumed that the node can't be referring to another node within the same file, however from testing it looks like that's not actually the case, because we hit the same code path when an external import to the same symbol exists already.
@crisbeto crisbeto mentioned this pull request Feb 7, 2023
Closed
crisbeto added a commit to crisbeto/angular that referenced this pull request Feb 7, 2023
@crisbeto
fix(compiler-cli): incorrectly detecting forward refs when symbol alr…
43b614c 
…eady exists in file

In angular#48898 the `isForwardRef` flag was added to indicate whether a reference should be wrapped in a `forwardRef`. This logic assumed that the node can't be referring to another node within the same file, however from testing it looks like that's not actually the case, because we hit the same code path when an external import to the same symbol exists already.
atscott pushed a commit that referenced this pull request Feb 7, 2023
@crisbeto @atscott
fix(compiler-cli): incorrectly detecting forward refs when symbol alr…
0cf1116 
…eady exists in file (#48988)

In #48898 the `isForwardRef` flag was added to indicate whether a reference should be wrapped in a `forwardRef`. This logic assumed that the node can't be referring to another node within the same file, however from testing it looks like that's not actually the case, because we hit the same code path when an external import to the same symbol exists already.

PR Close #48988
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Feb 23, 2023
Update angular monorepo to v15.2.0 (#1793)
43c700e 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fanimations/15.1.5/15.2.0) |
| [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcommon/15.1.5/15.2.0) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/15.1.5/15.2.0) |
| [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/15.1.5/15.2.0) |
| [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcore/15.1.5/15.2.0) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fforms/15.1.5/15.2.0) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/15.1.5/15.2.0) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/15.1.5/15.2.0) |

---

### Release Notes

<details>
<summary>angular/angular</summary>

### [`v15.2.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1520-2023-02-22)

[Compare Source](angular/angular@15.1.5...15.2.0)

#### Deprecations

#####

-   Class and `InjectionToken` guards and resolvers are
    deprecated. Instead, write guards as plain JavaScript functions and
    inject dependencies with `inject` from `@angular/core`.

#####

| Commit | Type | Description |
| -- | -- | -- |
| [926c35f4ac](angular/angular@926c35f) | docs | Deprecate class and InjectionToken and resolvers ([#&#8203;47924](angular/angular#47924)) |

##### common

| Commit | Type | Description |
| -- | -- | -- |
| [54b24eb40f](angular/angular@54b24eb) | feat | Add loaderParams attribute to NgOptimizedImage ([#&#8203;48907](angular/angular#48907)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [0cf11167f1](angular/angular@0cf1116) | fix | incorrectly detecting forward refs when symbol already exists in file ([#&#8203;48988](angular/angular#48988)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [a154db8a81](angular/angular@a154db8) | feat | add ng generate schematic to convert declarations to standalone ([#&#8203;48790](angular/angular#48790)) |
| [345e737daa](angular/angular@345e737) | feat | add ng generate schematic to convert to standalone bootstrapping APIs ([#&#8203;48848](angular/angular#48848)) |
| [e7318fc758](angular/angular@e7318fc) | feat | add ng generate schematic to remove unnecessary modules ([#&#8203;48832](angular/angular#48832)) |

##### language-service

| Commit | Type | Description |
| -- | -- | -- |
| [4ae384fd61](angular/angular@4ae384f) | feat | Allow auto-imports of a pipe via quick fix when its selector is used, both directly and via reexports. ([#&#8203;48354](angular/angular#48354)) |
| [141333411e](angular/angular@1413334) | feat | Introduce a new NgModuleIndex, and use it to suggest re-exports. ([#&#8203;48354](angular/angular#48354)) |
| [d0145033bd](angular/angular@d014503) | fix | generate forwardRef for same file imports ([#&#8203;48898](angular/angular#48898)) |

##### migrations

| Commit | Type | Description |
| -- | -- | -- |
| [2796230e95](angular/angular@2796230) | fix | add `enum` in `mode` option in `standalone` schema ([#&#8203;48851](angular/angular#48851)) |
| [816e76a578](angular/angular@816e76a) | fix | automatically prune root module after bootstrap step ([#&#8203;49030](angular/angular#49030)) |
| [bdbf21d04b](angular/angular@bdbf21d) | fix | avoid generating imports with forward slashes ([#&#8203;48993](angular/angular#48993)) |
| [32cf4e5cb9](angular/angular@32cf4e5) | fix | avoid internal modules when generating imports ([#&#8203;48958](angular/angular#48958)) |
| [521ccfbe6c](angular/angular@521ccfb) | fix | avoid interrupting the migration if language service lookup fails ([#&#8203;49010](angular/angular#49010)) |
| [a40cd47aa7](angular/angular@a40cd47) | fix | avoid modifying testing modules without declarations ([#&#8203;48921](angular/angular#48921)) |
| [1afa6ed322](angular/angular@1afa6ed) | fix | don't add ModuleWithProviders to standalone test components ([#&#8203;48987](angular/angular#48987)) |
| [c98c6a8452](angular/angular@c98c6a8) | fix | don't copy animations modules into the imports of test components ([#&#8203;49147](angular/angular#49147)) |
| [8389557848](angular/angular@8389557) | fix | don't copy unmigrated declarations into imports array ([#&#8203;48882](angular/angular#48882)) |
| [f82bdc4b01](angular/angular@f82bdc4) | fix | don't delete classes that may provide dependencies transitively ([#&#8203;48866](angular/angular#48866)) |
| [759db12e0b](angular/angular@759db12) | fix | duplicated comments on migrated classes ([#&#8203;48966](angular/angular#48966)) |
| [ba38178d19](angular/angular@ba38178) | fix | generate forwardRef for same file imports ([#&#8203;48898](angular/angular#48898)) |
| [03fcb36cfd](angular/angular@03fcb36) | fix | migrate HttpClientModule to provideHttpClient() ([#&#8203;48949](angular/angular#48949)) |
| [2de6dae16d](angular/angular@2de6dae) | fix | migrate RouterModule.forRoot with a config object to use features ([#&#8203;48935](angular/angular#48935)) |
| [770191cf1f](angular/angular@770191c) | fix | migrate tests when switching to standalone bootstrap API ([#&#8203;48987](angular/angular#48987)) |
| [c7926b5773](angular/angular@c7926b5) | fix | move standalone migrations into imports ([#&#8203;48987](angular/angular#48987)) |
| [65c74ed93e](angular/angular@65c74ed) | fix | normalize paths to posix ([#&#8203;48850](angular/angular#48850)) |
| [6377487b1a](angular/angular@6377487) | fix | only exclude bootstrapped declarations from initial standalone migration ([#&#8203;48987](angular/angular#48987)) |
| [e9e4449a43](angular/angular@e9e4449) | fix | preserve tsconfig in standalone migration ([#&#8203;48987](angular/angular#48987)) |
| [ffad1b49d9](angular/angular@ffad1b4) | fix | reduce number of files that need to be checked ([#&#8203;48987](angular/angular#48987)) |
| [ba7a757cc5](angular/angular@ba7a757) | fix | return correct alias when conflicting import exists ([#&#8203;49139](angular/angular#49139)) |
| [49a7c9f94a](angular/angular@49a7c9f) | fix | standalone migration incorrectly throwing path error for multi app projects ([#&#8203;48958](angular/angular#48958)) |
| [584976e6c8](angular/angular@584976e) | fix | support --defaults in standalone migration ([#&#8203;48921](angular/angular#48921)) |
| [03f47ac901](angular/angular@03f47ac) | fix | use consistent quotes in generated imports ([#&#8203;48876](angular/angular#48876)) |
| [ebae506d89](angular/angular@ebae506) | fix | use import remapper in root component ([#&#8203;49046](angular/angular#49046)) |
| [40c976c909](angular/angular@40c976c) | fix | use NgForOf instead of NgFor ([#&#8203;49022](angular/angular#49022)) |
| [4ac25b2aff](angular/angular@4ac25b2) | perf | avoid re-traversing nodes when resolving bootstrap call dependencies ([#&#8203;49010](angular/angular#49010)) |
| [26cb7ab2e6](angular/angular@26cb7ab) | perf | speed up language service lookups ([#&#8203;49010](angular/angular#49010)) |

##### platform-browser

| Commit | Type | Description |
| -- | -- | -- |
| [bf4ad38117](angular/angular@bf4ad38) | fix | remove styles from DOM of destroyed components ([#&#8203;48298](angular/angular#48298)) |

##### platform-server

| Commit | Type | Description |
| -- | -- | -- |
| [25e220a23a](angular/angular@25e220a) | fix | avoid duplicate TransferState info after renderApplication call ([#&#8203;49094](angular/angular#49094)) |

##### router

| Commit | Type | Description |
| -- | -- | -- |
| [31b94c762f](angular/angular@31b94c7) | feat | Add a withNavigationErrorHandler feature to provideRouter ([#&#8203;48551](angular/angular#48551)) |
| [dedac8d3f7](angular/angular@dedac8d) | feat | Add test helper for trigger navigations in tests ([#&#8203;48552](angular/angular#48552)) |

#### Special Thanks

Alan Agius, Alex Castle, Alex Rickabaugh, Andrew Kushnir, Andrew Scott, Dylan Hunn, Ikko Eltociear Ashimine, Ilyass, Jessica Janiuk, Joey Perrott, John Manners, Kalbarczyk, Kristiyan Kostadinov, Matthieu Riegler, Paul Gschwendtner, Pawel Kozlowski, Virginia Dooley, Walid Bouguima, cexbrayat and mgechev

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNDkuMCIsInVwZGF0ZWRJblZlciI6IjM0LjE1Mi4wIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1793
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@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 Mar 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@atscott atscott atscott approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler area: language-service Issues related to Angular's VS Code language service area: migrations Issues related to `ng update` migrations target: minor This PR is targeted for the next minor release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@crisbeto @dylhunn @atscott