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[devtools/ci]: fixed incorrect condition calculation for @reactVersion annotation #26997

Merged
merged 1 commit into from
Jun 23, 2023

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Jun 23, 2023

Suppose that you have this setup for devtools test:

// @reactVersion <= 18.1
// @reactVersion >= 17.1

With previous implementation, the accumulated condition will be "<= 18.1" && ">= 17.1", which is just ">= 17.1", when evaluated. That's why we executed some tests for old versions of react on main (and failed).

With these changes the resulting condition will be "<= 18.1 >= 17.1", not using &&, because semver does not support this operator. All currently failing tests will be skipped now as expected.

Also increased timeout value for shell server to start

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 23, 2023
@react-sizebot
Copy link

react-sizebot commented Jun 23, 2023

Comparing: 70e998a...34bfa13

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 164.56 kB 164.56 kB = 51.83 kB 51.83 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.97 kB 171.97 kB = 54.06 kB 54.06 kB
facebook-www/ReactDOM-prod.classic.js = 571.74 kB 571.74 kB = 100.83 kB 100.83 kB
facebook-www/ReactDOM-prod.modern.js = 555.52 kB 555.52 kB = 98.02 kB 98.02 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 34bfa13

@hoxyq hoxyq force-pushed the devtools/fix-ci-tests branch 2 times, most recently from e77934e to 9f98e21 Compare June 23, 2023 14:47
@hoxyq hoxyq merged commit 8ec962d into facebook:main Jun 23, 2023
@hoxyq hoxyq deleted the devtools/fix-ci-tests branch June 23, 2023 15:45
hoxyq added a commit that referenced this pull request Jun 26, 2023
…est (#27008)

- Correctly gate the test to `[18.0.0, 18.2.0]` versions of react, as it
was initially defined before
#26742
- I have recently fixed the gating logic in
#26955 and
#26997, should be stable now
- I have added a non-gated version of this test, which should run only
for the current version of react, the one we build from sources
- The test version for react `v[18.0.0, 18.2.0]` should expect
`"priorityLevel": "Immediate"`, the other `"priorityLevel": "Normal"`,
as it was changed in #26512
hoxyq added a commit that referenced this pull request Jul 4, 2023
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [#27032](#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[#27008](#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[#26997](#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[#26955](#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[#26963](#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[#26983](#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[#26951](#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[#26887](#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[#26947](#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[#25510](#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[#26888](#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[#26830](#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [#26825](#26825))
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…sion annotation (facebook#26997)

Suppose that you have this setup for devtools test:
```
// @reactVersion <= 18.1
// @reactVersion >= 17.1
```

With previous implementation, the accumulated condition will be `"<=
18.1" && ">= 17.1"`, which is just `">= 17.1"`, when evaluated. That's
why we executed some tests for old versions of react on main (and
failed).

With these changes the resulting condition will be `"<= 18.1 >= 17.1"`,
not using `&&`, because semver does not support this operator. All
currently failing tests will be skipped now as expected.

Also increased timeout value for shell server to start
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…est (facebook#27008)

- Correctly gate the test to `[18.0.0, 18.2.0]` versions of react, as it
was initially defined before
facebook#26742
- I have recently fixed the gating logic in
facebook#26955 and
facebook#26997, should be stable now
- I have added a non-gated version of this test, which should run only
for the current version of react, the one we build from sources
- The test version for react `v[18.0.0, 18.2.0]` should expect
`"priorityLevel": "Immediate"`, the other `"priorityLevel": "Normal"`,
as it was changed in facebook#26512
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
List of changes:
* Devtools:-Removed unused CSS ([Biki-das](https://github.com/Biki-das)
in [facebook#27032](facebook#27032))
* fix[devtools/profilingCache-test]: specify correct version gate for
test ([hoxyq](https://github.com/hoxyq) in
[facebook#27008](facebook#27008))
* fix[devtools/ci]: fixed incorrect condition calculation for
@reactVersion annotation ([hoxyq](https://github.com/hoxyq) in
[facebook#26997](facebook#26997))
* fix[ci]: fixed jest configuration not to skip too many devtools tests
([hoxyq](https://github.com/hoxyq) in
[facebook#26955](facebook#26955))
* fix[devtools/standalone]: update webpack configurations
([hoxyq](https://github.com/hoxyq) in
[facebook#26963](facebook#26963))
* fix[devtools]: check if fiber is unmounted before trying to highlight
([hoxyq](https://github.com/hoxyq) in
[facebook#26983](facebook#26983))
* feat[devtools]: support x_google_ignoreList source maps extension
([hoxyq](https://github.com/hoxyq) in
[facebook#26951](facebook#26951))
* chore[devtools]: upgrade to webpack v5
([hoxyq](https://github.com/hoxyq) in
[facebook#26887](facebook#26887))
* fix[devtools]: display NaN as string in values
([hoxyq](https://github.com/hoxyq) in
[facebook#26947](facebook#26947))
* fix: devtools cannot be closed correctly
([Jack-Works](https://github.com/Jack-Works) in
[facebook#25510](facebook#25510))
* Fix:- Fixed dev tools inspect mode on Shadow dom
([Biki-das](https://github.com/Biki-das) in
[facebook#26888](facebook#26888))
* Updated copyright text to Copyright (c) Meta Platforms, Inc. and its …
([denmo530](https://github.com/denmo530) in
[facebook#26830](facebook#26830))
* Fix strict mode badge URL ([ibrahemid](https://github.com/ibrahemid)
in [facebook#26825](facebook#26825))
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…sion annotation (#26997)

Suppose that you have this setup for devtools test:
```
// @reactVersion <= 18.1
// @reactVersion >= 17.1
```

With previous implementation, the accumulated condition will be `"<=
18.1" && ">= 17.1"`, which is just `">= 17.1"`, when evaluated. That's
why we executed some tests for old versions of react on main (and
failed).

With these changes the resulting condition will be `"<= 18.1 >= 17.1"`,
not using `&&`, because semver does not support this operator. All
currently failing tests will be skipped now as expected.

Also increased timeout value for shell server to start

DiffTrain build for commit 8ec962d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants