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

fix(migrations): avoid migrating the same class multiple times in standalone migration #49245

Closed
wants to merge 1 commit into from
Closed

fix(migrations): avoid migrating the same class multiple times in standalone migration #49245

wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

If a class is declared in multiple modules, the standalone migration may end up generating invalid code. While declaring a class in multiple modules is an error, it can happen with modules in tests. These changes avoid the issue by using a Set to track the classes being migrated.

@crisbeto
fix(migrations): avoid migrating the same class multiple times in sta…
691e590 
…ndalone migration

If a class is declared in multiple modules, the standalone migration may end up generating invalid code. While declaring a class in multiple modules is an error, it can happen with modules in tests. These changes avoid the issue by using a `Set` to track the classes being migrated.
@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: migrations Issues related to `ng update` migrations labels Feb 28, 2023
@ngbot ngbot bot modified the milestone: Backlog Feb 28, 2023
crisbeto
crisbeto commented Feb 28, 2023
View reviewed changes
packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
}
}

testObjectsToMigrate.push(...testObjects);
testObjects.forEach(obj => testObjectsToMigrate.add(obj));
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that I didn't add a test for this case, because the way the string buffer works from the CLI means that the two conflicting edits of the same class are valid. This issue came up internally where we use a different way of manipulating strings.

@crisbeto crisbeto 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 28, 2023
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 87affad.

AndrewKushnir pushed a commit that referenced this pull request Feb 28, 2023
@crisbeto @AndrewKushnir
fix(migrations): avoid migrating the same class multiple times in sta…
44d095a 
…ndalone migration (#49245)

If a class is declared in multiple modules, the standalone migration may end up generating invalid code. While declaring a class in multiple modules is an error, it can happen with modules in tests. These changes avoid the issue by using a `Set` to track the classes being migrated.

PR Close #49245
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 29, 2023
Update angular monorepo (#1801)
6e52606 
This PR contains the following updates:

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

---

### Release Notes

<details>
<summary>angular/angular (@&#8203;angular/animations)</summary>

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

[Compare Source](angular/angular@15.2.3...15.2.4)

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [bae6b5ceb1](angular/angular@bae6b5c) | fix | Allow `TestBed.configureTestingModule` to work with recursive cycle of standalone components. ([#&#8203;49473](angular/angular#49473)) |
| [087f4412af](angular/angular@087f441) | fix | more accurate matching of classes during content projection ([#&#8203;48888](angular/angular#48888)) |

#### Special Thanks

Aditya Srinivasan, Alex Rickabaugh, Andrew Scott, Kristiyan Kostadinov, Masaoki Kobayashi, Matthieu Riegler, Paul Gschwendtner, Peter Götz, Thomas Pischke, Virginia Dooley and avmaxim

<!-- CHANGELOG SPLIT MARKER -->

### [`v15.2.3`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1523-2023-03-16)

[Compare Source](angular/angular@15.2.2...15.2.3)

#### Special Thanks

Alan Agius, Esteban Gehring, Matthieu Riegler and Virginia Dooley

<!-- CHANGELOG SPLIT MARKER -->

### [`v15.2.2`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1522-2023-03-08)

[Compare Source](angular/angular@15.2.1...15.2.2)

##### migrations

| Commit | Type | Description |
| -- | -- | -- |
| [6207d6f1f0](angular/angular@6207d6f) | fix | add protractor support if protractor imports are detected ([#&#8203;49274](angular/angular#49274)) |

#### Special Thanks

Alan Agius, Andrew Kushnir, Andrew Scott, Kristiyan Kostadinov, Matthieu Riegler, Paul Gschwendtner, Sai Kartheek Bommisetty and Vinit Neogi

<!-- CHANGELOG SPLIT MARKER -->

### [`v15.2.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1521-2023-03-01)

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

##### common

| Commit | Type | Description |
| -- | -- | -- |
| [f0e926074d](angular/angular@f0e9260) | fix | make Location.normalize() return the correct path when the base path contains characters that interfere with regex syntax. ([#&#8203;49181](angular/angular#49181)) |

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [04d8b6c61a](angular/angular@04d8b6c) | fix | do not persist component analysis if template/styles are missing ([#&#8203;49184](angular/angular#49184)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [d60ea6ab5a](angular/angular@d60ea6a) | fix | update zone.js peerDependencies ranges ([#&#8203;49244](angular/angular#49244)) |

##### migrations

| Commit | Type | Description |
| -- | -- | -- |
| [44d095a61c](angular/angular@44d095a) | fix | avoid migrating the same class multiple times in standalone migration ([#&#8203;49245](angular/angular#49245)) |
| [92b0bda9e4](angular/angular@92b0bda) | fix | delete barrel exports in standalone migration ([#&#8203;49176](angular/angular#49176)) |

##### router

| Commit | Type | Description |
| -- | -- | -- |
| [3062442728](angular/angular@3062442) | fix | add error message when using loadComponent with a NgModule ([#&#8203;49164](angular/angular#49164)) |

#### Special Thanks

Alan Agius, Andrew Kushnir, Aristeidis Bampakos, Craig Spence, Doug Parker, Iván Navarro, Joey Perrott, Kristiyan Kostadinov, Matthieu Riegler, Michael Ziluck, Paul Gschwendtner, Stephanie Tuerk, Vincent and Virginia Dooley

<!-- CHANGELOG SPLIT MARKER -->

</details>

<details>
<summary>angular/angular (zone.js)</summary>

### [`v0.13.0`](angular/angular@zone.js-0.12.0...zone.js-0.13.0)

[Compare Source](angular/angular@zone.js-0.12.0...zone.js-0.13.0)

</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.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- 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:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNTQuMSIsInVwZGF0ZWRJblZlciI6IjM1LjI0LjUifQ==-->

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

@devversion devversion devversion approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: migrations Issues related to `ng update` migrations target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@crisbeto @AndrewKushnir @devversion