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(windows): Misc cleanup to windows. Support only RNW63 and newer. #1419

Merged
merged 7 commits into from Jun 2, 2022

Conversation

namrog84
Copy link
Collaborator

@namrog84 namrog84 commented Jun 1, 2022

Description

Changed to general version for WindowsTargetPlatformVersion

Using an explicit version of WindowsTargetPlatformVersion requires that specific windows SDK version to be installed on the machine.
In Visual Studio 2017 (version 15 or build tools 141) and earlier it was required but as of Visual Studio 2019 (v16 or v142) and Visual Studio 2022(v17 or v143) you can simply specify "10.0". This allows a lot more flexibility to the developer or build machine as there is a growing number of different windows 10 SDK out there now.

Primary changes in this PR.

  • Change WindowsTargetPlatformVersion to 10.0
  • Drop support of RNW61 and RNW62.
  • Drop support for deprecated arm 32 bit support on windows. Note still supports ARM64 bit.
  • Remove comments about manual linking.
  • Specifically only support MSVC build tools v142 and v143 (VS2019 and VS2022) and newer.
  • Misc auto formatting changed.
  • Alphabeticalized the pch.h file.

Compatibility

OS Implemented
Windows
  • I have tested this on a device/simulator for each compatible OS
  • I added the documentation in README.md
  • I updated the typings files (privateTypes.ts, types.ts)
  • I added a sample use of the API (example/App.js)

Misc notes: Newer versions of azure hosted windows images have a different subset of windows 10 sdk version installed.
VS2022 Azure Window image has
10.0.17763.0, 10.0.19041.0, 10.0.20348.0, 10.0.22000.0
VS2019 Azure Window image had
10.0.14393.0, 10.0.16299.0, 10.0.17134.0, 10.0.17763.0, 10.0.18362.0, 10.0.19041.0, 10.0.20348.0, 10.0.22000.0.

BREAKING CHANGE: Windows only: Dropping support of MSVC build tools of v141 (VS2017) and lower. To MSVC build tools v142 (VS2019) and newer, no impact.
BREAKING CHANGE: Windows only: Dropping support of ARM 32 bit. No impact to x86, x64, or arm64.

@namrog84 namrog84 changed the title Change WindowsTargetPlatformVersion to 10.0 fix(windows) Change WindowsTargetPlatformVersion to 10.0 Jun 1, 2022
@namrog84 namrog84 changed the title fix(windows) Change WindowsTargetPlatformVersion to 10.0 fix(windows): Change WindowsTargetPlatformVersion to 10.0 Jun 1, 2022
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @namrog84 - and I have noticed the nit-pickyness on SDK versions required when I test RNWindows here, and they do bloat up the storage space taken, this will be nice

Am I correct in interpreting that this will actually break people that are using old tools?
I think I've mentioned my breaking change policy before but to be clear, breaking changes are fine for me - version numbers are free, as a dev I just want to know exactly what broke, who it affects, and what to do, and I never want breaks without warning

So I think this is a breaking change release but it gets a note that 99.999% of people will love, something like:

BREAKING CHANGE: If you are using react-native-windows, and you have Visual Studio versions older than Visual Studio 2019 (v16 or v142) and Visual Studio 2022 (v17 or v143), you must upgrade Visual Studio to use this version. No one else is affected.

?

@namrog84
Copy link
Collaborator Author

namrog84 commented Jun 1, 2022

That is correct this is technically a breaking change for people using old MSVC build tools that come with Visual Studio 2017 or Visual Studio 2015. It is non breaking for build tools that come with Visual Studio 2019 or 2022. Assuming other things don't break in RNW

Am I correct in interpreting that this will actually break people that are using old tools?

Though to be super specific, it technically isn't Visual Studio that is breaking change but the underlying MSVC build tools that comes bundled with each of those visual studio versions (e.g. Visual Studio 2022 comes with MSVC v143), and it is possible to install older build tools with newer visual studio during 'transitional' periods.

I think it's fair to say most people doing react-native-windows related work should be on at least VS2019 (MSVC v142) as that's what RNW has been using/recommending for a number of years now. And VS2022 has been out for almost a year soon too.

BREAKING CHANGE: If you are using react-native-windows, and you have Visual Studio versions using build tools older than MSVC v142(2019), you must upgrade Visual Studio and/or your build tools to use at least MSVC v142 (Visual Studio 2019). No one else is affected.

Copy link
Collaborator Author

@namrog84 namrog84 left a comment

Choose a reason for hiding this comment

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

@mikehardy it appears like visual studio 16(2019) is already defined as minimum version here. But one could still technically install older build tools with it.

I am on phone and not sure if its tagging right. Line 12 of the vcxprog file is what I was referencing

16.

@namrog84
Copy link
Collaborator Author

namrog84 commented Jun 1, 2022

Also if going to consider as breaking change. I can do some other things that likely need doing that also sorta fall into breaking change category.

Would you prefer me doing separate pr or add to this one then?

@mikehardy
Copy link
Collaborator

I think this is small enough (it's one line really!) you can do them as a batch here if you like. And I do think this is a breaking change just to make certain we don't blow anyone up - as mentioned I never mind seeing a major version as long as the messaging is specific, and when it doesn't affect me, great

Whatever you've got go ahead - just do your best to define what the "I'm affected" criteria are for each one, and we'll have happy devs as they'll all have the knowledge they need

@namrog84
Copy link
Collaborator Author

namrog84 commented Jun 1, 2022

Any objections to removing the weirdness of supporting RNW61 and RNW62. Considering those are pretty old and had some wonkiness around them compared to RN63-RN68.

@mikehardy
Copy link
Collaborator

I don't see a problem. Clearly document, bump the version, move on. Those are very very old versions in my opinion and are not really supportable for ios (I'd say 65 is a minimum iOS really, you're just hacking your build toolchain to do anything lower) and probably up to 67 or 68 for android shortly because of need for API31. No one is forced to upgrade

So go for it yeah

@namrog84 namrog84 changed the title fix(windows): Change WindowsTargetPlatformVersion to 10.0 fix(windows): Misc cleanup to windows. Support only RNW63 and newer. Jun 1, 2022
@namrog84
Copy link
Collaborator Author

namrog84 commented Jun 1, 2022

Updated original PR description. Bumped the version to 9.0.0 as well.
Did I miss anything?

  • Changed Windows to use WindowsTargetPlatformVersion 10.0 (latest installed version) (Windows SDK version).
  • BREAKING CHANGE: Windows only: Dropping support of MSVC build tools of v141 (VS2017) and lower. To MSVC build tools v142 (VS2019) and newer, no impact.
  • BREAKING CHANGE: Windows only: Dropping support of ARM 32 bit. No impact to x86, x64, or arm64.

package.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

This looks great, just a question on docs

windows/README.md Outdated Show resolved Hide resolved
@mikehardy mikehardy added pending-merge looks good, waiting on CI or similar feedback-requested labels Jun 2, 2022
Co-authored-by: Mike Hardy <github@mikehardy.net>
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

I think it's all corner cases, probably no one affected but I've been burnt on breaking changes before (both as a maintainer and a dev) so I'm just all nit-picky about it now. This is all good to go then, thanks

@mikehardy mikehardy merged commit 3bd6f88 into react-native-device-info:master Jun 2, 2022
@mikehardy mikehardy removed the pending-merge looks good, waiting on CI or similar label Jun 2, 2022
rndi-bot pushed a commit that referenced this pull request Jun 2, 2022
# [9.0.0](v8.7.1...v9.0.0) (2022-06-02)

* fix(windows)!: Misc cleanup to windows. Support only RNW63 and newer. (#1419) ([3bd6f88](3bd6f88)), closes [#1419](#1419)

### BREAKING CHANGES

* needs react-native-windows 0.63+ and MSVC build tools v142+ (Visual Studio 2019+), drop arm32
@rndi-bot
Copy link
Collaborator

rndi-bot commented Jun 2, 2022

🎉 This PR is included in version 9.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mikehardy
Copy link
Collaborator

So I was on windows this weekend (usually I'm linux and just have VMs) with this project handy, I updated it and attempted to build it after uninstalling Visual Studio Community 2019 completely, downloading Visual Studio Community 2022 and running the react-native-windows "development dependencies" script in elevated powershell.

It tells me I have everything up to date before it finishes
I am in sync with HEAD on master here
I open the solution here in Visual Studio Community 2022 and it tells me that I don't have Windows 10 SDK 10.0.18362.0 installed, and I should retarget
If I retarget it tells me that it is set to "Windows 10 (latest installed)"
If I check Visual Studio Community 2022 installer, it indicates I have WIndows 10 SDKs 10.0.19041.0 and 10.0.20348.0 installed, so I'm confused, what's missing?

I don't want to install an old SDK for no reason, and it was my understanding that this PR would make it work out of the box with whatever Windows 10 SDK you had?

I'm like a college freshman with windows development so if you can help, please explain it to me slowly in small words ;-)

@namrog84
Copy link
Collaborator Author

namrog84 commented Jun 4, 2022

You shouldn't have seen those prompts or have to install any other windows 10 sdk versions. Let me investigate...

I am on a different computer today (Saturday) and I didn't even have npm or yarn installed so everything is pretty fresh here.
Installed all that, and I installed the 2 versions you implied you had.

https://i.imgur.com/NDxEpcs.png

I was able to open and launch without any issues.

https://i.imgur.com/JEtmkgg.png

However, there is 1 piece of code that Visual Studio could be causing the issue, that I can do a PR for.

In mean time, do a git clean -fdx and try again. And hit cancel on any retargeting(though I suspect after a git clean -fdx it won't ask). Form inside VS2022, Right click example, and go to properties, then look in configuration properties -> general, and see what the default target platform version is. https://i.imgur.com/zTME19N.png

I suspect what was happening you were using a git clone with some .vs cached files (gitignored) from a while ago, and it saved an old target platform version from last time, and when you opened it again, it still had some 'undefined' values that had set to the old windows 10 sdk version.

@namrog84
Copy link
Collaborator Author

namrog84 commented Jun 4, 2022

Added PR #1424 to have the example project explicitly use latest windows 10 sdk to minimize any stale 'cache' issues.

@mikehardy
Copy link
Collaborator

I think this was an own goal though #1424 should help.
Silly me, I had forgotten to do the dev-copy-windows and example-install-windows here, so naturally it was all out of whack

image

The operating system versioning is a little silly I think - the phrase "Microsoft Windows 11 Home" is never actually included anywhere even though it's the first item in system information app, but then is reported there (and here) as 10.0.22000 because that makes sense 😆

Anyway, I think this was my fault and I'm sorry if it took much of your time

WictorWilnd added a commit to WictorWilnd/react-native-device-info that referenced this pull request Feb 27, 2023
# [9.0.0](react-native-device-info/react-native-device-info@v8.7.1...v9.0.0) (2022-06-02)

* fix(windows)!: Misc cleanup to windows. Support only RNW63 and newer. (#1419) ([3937e30](react-native-device-info/react-native-device-info@3937e30)), closes [#1419](react-native-device-info/react-native-device-info#1419)

### BREAKING CHANGES

* needs react-native-windows 0.63+ and MSVC build tools v142+ (Visual Studio 2019+), drop arm32
wdavis122 added a commit to wdavis122/react-native-device-info that referenced this pull request Jun 6, 2024
# [9.0.0](react-native-device-info/react-native-device-info@v8.7.1...v9.0.0) (2022-06-02)

* fix(windows)!: Misc cleanup to windows. Support only RNW63 and newer. (#1419) ([3bd6f88](react-native-device-info/react-native-device-info@3bd6f88)), closes [#1419](react-native-device-info/react-native-device-info#1419)

### BREAKING CHANGES

* needs react-native-windows 0.63+ and MSVC build tools v142+ (Visual Studio 2019+), drop arm32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants