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(cli): fix cache not being used when there are no dependencies #9522

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

tido64
Copy link
Member

@tido64 tido64 commented Feb 11, 2022

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Why

This addresses a perf regression in autolinking when there are no dependencies to link.

Resolves #9518

What

Instead of checking whether the dependencies cache is empty, check whether it was instantiated.

Testing

Before After
22488ms 3561ms
Microsoft Reviewers: Open in CodeFlow

@NickGerleman
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tido64
Copy link
Member Author

tido64 commented Feb 11, 2022

Looks like Beachball is having issues again:

ERROR: Attempting to bump to a version that already exists in the registry: @office-iss/react-native-win32@0.0.0-canary.133

ERROR: Attempting to bump to a version that already exists in the registry: @react-native-windows/virtualized-list@0.0.0-canary.29
Validating package version - react-native-windows-init@1.1.86 OK!
Validating package version - react-native-windows@0.0.0-canary.454 OK!
Validating package version - @react-native-windows/automation-channel@0.1.28 OK!
Validating package version - @react-native-windows/automation@0.1.39 OK!
Validating package version - @react-native-windows/automation-commands@0.0.66 OK!
Something went wrong with the publish! Manually update these package and versions:
- @react-native-windows/cli@0.0.0-canary.115
- react-native-windows-init@1.1.86
- react-native-windows@0.0.0-canary.454
- @react-native-windows/automation-channel@0.1.28
- @react-native-windows/automation@0.1.39
- @react-native-windows/automation-commands@0.0.66
- e2e-test-app@0.0.0
- integration-test-app@0.0.0
- playground@0.0.54
- sample-apps@0.0.0
- @office-iss/react-native-win32@0.0.0-canary.133
- @office-iss/react-native-win32-tester@0.0.1
- @react-native-windows/tester@0.0.1
- @react-native-windows/virtualized-list@0.0.0-canary.29

@NickGerleman
Copy link
Collaborator

Looks like Beachball is having issues again:

ERROR: Attempting to bump to a version that already exists in the registry: @office-iss/react-native-win32@0.0.0-canary.133

ERROR: Attempting to bump to a version that already exists in the registry: @react-native-windows/virtualized-list@0.0.0-canary.29
Validating package version - react-native-windows-init@1.1.86 OK!
Validating package version - react-native-windows@0.0.0-canary.454 OK!
Validating package version - @react-native-windows/automation-channel@0.1.28 OK!
Validating package version - @react-native-windows/automation@0.1.39 OK!
Validating package version - @react-native-windows/automation-commands@0.0.66 OK!
Something went wrong with the publish! Manually update these package and versions:
- @react-native-windows/cli@0.0.0-canary.115
- react-native-windows-init@1.1.86
- react-native-windows@0.0.0-canary.454
- @react-native-windows/automation-channel@0.1.28
- @react-native-windows/automation@0.1.39
- @react-native-windows/automation-commands@0.0.66
- e2e-test-app@0.0.0
- integration-test-app@0.0.0
- playground@0.0.54
- sample-apps@0.0.0
- @office-iss/react-native-win32@0.0.0-canary.133
- @office-iss/react-native-win32-tester@0.0.1
- @react-native-windows/tester@0.0.1
- @react-native-windows/virtualized-list@0.0.0-canary.29

This specific one might be our fault. We have a known race condition in our CI logic that can cause PRs to fail when run around the same time as our nightly publish. It is unexpected though, since this was not close to that time. Another case where we have seen these issues is if Beachball were to run into issues during publish, and not push results to our repository, but it looks like it did correctly push that.

Need to take a closer look.

@NickGerleman
Copy link
Collaborator

Seeing the same Beachball issue with #9491. Doing a beachball bump --verbose on a local branch merged to main, it seems to be calculating some packages to be bumped that are already up-to-date (with package.json up to date as well). beachball sync sees that everything locally is correct as well. So something seems to be going wrong during the actual bumping process evaluation I think?

@NickGerleman
Copy link
Collaborator

NickGerleman commented Feb 11, 2022

I think I've identified the issue, with c3d84b0.

@NickGerleman
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonthysell jonthysell merged commit 564f0ce into main Feb 11, 2022
@tido64 tido64 deleted the tido/fix-dependencies-cache branch February 11, 2022 23:07
jonthysell pushed a commit to jonthysell/react-native-windows that referenced this pull request Feb 15, 2022
This PR backports microsoft#9522 to 0.68.

- Bug fix (non-breaking change which fixes an issue)
This addresses a perf regression in autolinking when there are no dependencies to link.

Resolves microsoft#9518

Instead of checking whether the dependencies cache is empty, check whether it was instantiated.

| Before | After |
| -: | -: |
| 22488ms | 3561ms |
jonthysell pushed a commit to jonthysell/react-native-windows that referenced this pull request Feb 15, 2022
This PR backports microsoft#9522 to 0.67.

- Bug fix (non-breaking change which fixes an issue)
This addresses a perf regression in autolinking when there are no dependencies to link.

Resolves microsoft#9518

Instead of checking whether the dependencies cache is empty, check whether it was instantiated.

| Before | After |
| -: | -: |
| 22488ms | 3561ms |
jonthysell pushed a commit to jonthysell/react-native-windows that referenced this pull request Feb 15, 2022
This PR backports microsoft#9522 to 0.66.

- Bug fix (non-breaking change which fixes an issue)
This addresses a perf regression in autolinking when there are no dependencies to link.

Resolves microsoft#9518

Instead of checking whether the dependencies cache is empty, check whether it was instantiated.

| Before | After |
| -: | -: |
| 22488ms | 3561ms |
ghost pushed a commit that referenced this pull request Feb 15, 2022
…ies (#9549)

This PR backports #9522 to 0.67.

- Bug fix (non-breaking change which fixes an issue)
This addresses a perf regression in autolinking when there are no dependencies to link.

Resolves #9518

Instead of checking whether the dependencies cache is empty, check whether it was instantiated.

| Before | After |
| -: | -: |
| 22488ms | 3561ms |

Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
ghost pushed a commit that referenced this pull request Feb 15, 2022
…ies (#9548)

This PR backports #9522 to 0.68.

- Bug fix (non-breaking change which fixes an issue)
This addresses a perf regression in autolinking when there are no dependencies to link.

Resolves #9518

Instead of checking whether the dependencies cache is empty, check whether it was instantiated.

| Before | After |
| -: | -: |
| 22488ms | 3561ms |

Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
ghost pushed a commit that referenced this pull request Feb 15, 2022
…ies (#9550)

This PR backports #9522 to 0.66.

- Bug fix (non-breaking change which fixes an issue)
This addresses a perf regression in autolinking when there are no dependencies to link.

Resolves #9518

Instead of checking whether the dependencies cache is empty, check whether it was instantiated.

| Before | After |
| -: | -: |
| 22488ms | 3561ms |

Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

react-native autolink-windows has regressed in speed since 0.65
4 participants