From 1a7817125ba3709dd2bb7d9783267752359976df Mon Sep 17 00:00:00 2001 From: Matteo Prosperi Date: Fri, 4 Sep 2026 18:03:42 -0700 Subject: [PATCH] Migrate Node.js general options to Unified Settings Register the three legacy general options with full bidirectional string migration, refresh unmodified cached values before launch, and add focused compatibility tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Nodejs/Product/Nodejs/Nodejs.csproj | 8 + .../Options/NodejsGeneralOptionsPage.cs | 75 +++++- .../Nodejs/Project/NodejsProjectLauncher.cs | 7 +- Nodejs/Product/Nodejs/UnifiedSettings.pkgdef | 5 + .../NodejsGeneralOptions.registration.json | 95 ++++++++ Nodejs/Product/Nodejs/VSPackage.resx | 15 ++ .../Tests/Core/NodejsGeneralOptionsTests.cs | 219 ++++++++++++++++++ Nodejs/Tests/Core/NodejsTests.csproj | 11 + 8 files changed, 423 insertions(+), 12 deletions(-) create mode 100644 Nodejs/Product/Nodejs/UnifiedSettings.pkgdef create mode 100644 Nodejs/Product/Nodejs/UnifiedSettings/NodejsGeneralOptions.registration.json create mode 100644 Nodejs/Tests/Core/NodejsGeneralOptionsTests.cs diff --git a/Nodejs/Product/Nodejs/Nodejs.csproj b/Nodejs/Product/Nodejs/Nodejs.csproj index 28de1a8d6..1009639fd 100644 --- a/Nodejs/Product/Nodejs/Nodejs.csproj +++ b/Nodejs/Product/Nodejs/Nodejs.csproj @@ -312,6 +312,14 @@ PreserveNewest true + + PreserveNewest + true + + + PreserveNewest + true + diff --git a/Nodejs/Product/Nodejs/Options/NodejsGeneralOptionsPage.cs b/Nodejs/Product/Nodejs/Options/NodejsGeneralOptionsPage.cs index 87b957996..63ed05ede 100644 --- a/Nodejs/Product/Nodejs/Options/NodejsGeneralOptionsPage.cs +++ b/Nodejs/Product/Nodejs/Options/NodejsGeneralOptionsPage.cs @@ -14,6 +14,12 @@ public class NodejsGeneralOptionsPage : NodejsDialogPage private const string CheckForLongPathsSetting = "CheckForLongPaths"; private NodejsGeneralOptionsControl _window; + private bool _waitOnAbnormalExit; + private bool _waitOnNormalExit; + private bool _editAndContinue; + private bool _waitOnAbnormalExitModified; + private bool _waitOnNormalExitModified; + private bool _editAndContinueModified; public NodejsGeneralOptionsPage() : base("General") @@ -38,18 +44,42 @@ protected override IWin32Window Window /// True if Node processes should pause for input before exiting /// if they exit abnormally. /// - public bool WaitOnAbnormalExit { get; set; } + public bool WaitOnAbnormalExit + { + get => this._waitOnAbnormalExit; + set + { + this._waitOnAbnormalExit = value; + this._waitOnAbnormalExitModified = true; + } + } /// /// True if Node processes should pause for input before exiting /// if they exit normally. /// - public bool WaitOnNormalExit { get; set; } + public bool WaitOnNormalExit + { + get => this._waitOnNormalExit; + set + { + this._waitOnNormalExit = value; + this._waitOnNormalExitModified = true; + } + } /// /// Indicates whether Edit and Continue feature should be enabled. /// - public bool EditAndContinue { get; set; } + public bool EditAndContinue + { + get => this._editAndContinue; + set + { + this._editAndContinue = value; + this._editAndContinueModified = true; + } + } /// /// Resets settings back to their defaults. This should be followed by @@ -65,12 +95,36 @@ public override void ResetSettings() public override void LoadSettingsFromStorage() { - // Load settings from storage. - this.WaitOnAbnormalExit = LoadBool(WaitOnAbnormalExitSetting) ?? true; - this.WaitOnNormalExit = LoadBool(WaitOnNormalExitSetting) ?? false; - this.EditAndContinue = LoadBool(EditAndContinueSetting) ?? true; + this._waitOnAbnormalExit = LoadBool(WaitOnAbnormalExitSetting) ?? true; + this._waitOnNormalExit = LoadBool(WaitOnNormalExitSetting) ?? false; + this._editAndContinue = LoadBool(EditAndContinueSetting) ?? true; + this._waitOnAbnormalExitModified = false; + this._waitOnNormalExitModified = false; + this._editAndContinueModified = false; + + if (this._window != null) + { + this._window.SyncControlWithPageSettings(this); + } + } + + internal void RefreshSettingsFromStorage() + { + if (!this._waitOnAbnormalExitModified) + { + this._waitOnAbnormalExit = LoadBool(WaitOnAbnormalExitSetting) ?? true; + } + + if (!this._waitOnNormalExitModified) + { + this._waitOnNormalExit = LoadBool(WaitOnNormalExitSetting) ?? false; + } + + if (!this._editAndContinueModified) + { + this._editAndContinue = LoadBool(EditAndContinueSetting) ?? true; + } - // Synchronize UI with backing properties. if (this._window != null) { this._window.SyncControlWithPageSettings(this); @@ -79,16 +133,17 @@ public override void LoadSettingsFromStorage() public override void SaveSettingsToStorage() { - // Synchronize backing properties with UI. if (this._window != null) { this._window.SyncPageWithControlSettings(this); } - // Save settings. SaveBool(WaitOnNormalExitSetting, this.WaitOnNormalExit); SaveBool(WaitOnAbnormalExitSetting, this.WaitOnAbnormalExit); SaveBool(EditAndContinueSetting, this.EditAndContinue); + this._waitOnAbnormalExitModified = false; + this._waitOnNormalExitModified = false; + this._editAndContinueModified = false; } } } diff --git a/Nodejs/Product/Nodejs/Project/NodejsProjectLauncher.cs b/Nodejs/Product/Nodejs/Project/NodejsProjectLauncher.cs index fb777c2de..c79375c6d 100644 --- a/Nodejs/Product/Nodejs/Project/NodejsProjectLauncher.cs +++ b/Nodejs/Product/Nodejs/Project/NodejsProjectLauncher.cs @@ -136,10 +136,13 @@ private void StartNodeProcess(string file, string nodePath, bool shouldStartBrow psi.EnvironmentVariables[nameValue.Key] = nameValue.Value; } + var generalOptions = NodejsPackage.Instance.GeneralOptionsPage; + generalOptions.RefreshSettingsFromStorage(); + var process = NodeProcess.Start( psi, - waitOnAbnormal: NodejsPackage.Instance.GeneralOptionsPage.WaitOnAbnormalExit, - waitOnNormal: NodejsPackage.Instance.GeneralOptionsPage.WaitOnNormalExit); + waitOnAbnormal: generalOptions.WaitOnAbnormalExit, + waitOnNormal: generalOptions.WaitOnNormalExit); this._project.OnDispose += process.ResponseToTerminateEvent; diff --git a/Nodejs/Product/Nodejs/UnifiedSettings.pkgdef b/Nodejs/Product/Nodejs/UnifiedSettings.pkgdef new file mode 100644 index 000000000..4e05b7c9c --- /dev/null +++ b/Nodejs/Product/Nodejs/UnifiedSettings.pkgdef @@ -0,0 +1,5 @@ +// CacheTag must change whenever the Unified Settings manifest changes. +[$RootKey$\SettingsManifests\{FE8A8C3D-328A-476D-99F9-2A24B75F8C7F}] +@="Microsoft.NodejsTools.NodejsPackage" +"ManifestPath"="$PackageFolder$\UnifiedSettings\NodejsGeneralOptions.registration.json" +"CacheTag"=qword:08DE50FA23033A60 diff --git a/Nodejs/Product/Nodejs/UnifiedSettings/NodejsGeneralOptions.registration.json b/Nodejs/Product/Nodejs/UnifiedSettings/NodejsGeneralOptions.registration.json new file mode 100644 index 000000000..11e1d6114 --- /dev/null +++ b/Nodejs/Product/Nodejs/UnifiedSettings/NodejsGeneralOptions.registration.json @@ -0,0 +1,95 @@ +{ + "$schema": "https://aka.ms/unified-settings-experience/registration/schema", + "properties": { + "debugging.nodejs.general.waitOnAbnormalExit": { + "type": "boolean", + "title": "@UnifiedSettings_WaitOnAbnormalExit;{FE8A8C3D-328A-476D-99F9-2A24B75F8C7F}", + "default": true, + "order": 0, + "migration": { + "custom": { + "mode": "full", + "inputs": [ + { + "store": "VsUserSettingsRegistry", + "path": "NodejsTools\\Options\\General\\WaitOnAbnormalExit" + } + ], + "map": [ + { + "result": true, + "matches": [ "True" ] + }, + { + "result": false, + "matches": [ "False" ] + } + ] + } + } + }, + "debugging.nodejs.general.waitOnNormalExit": { + "type": "boolean", + "title": "@UnifiedSettings_WaitOnNormalExit;{FE8A8C3D-328A-476D-99F9-2A24B75F8C7F}", + "default": false, + "order": 1, + "migration": { + "custom": { + "mode": "full", + "inputs": [ + { + "store": "VsUserSettingsRegistry", + "path": "NodejsTools\\Options\\General\\WaitOnNormalExit" + } + ], + "map": [ + { + "result": true, + "matches": [ "True" ] + }, + { + "result": false, + "matches": [ "False" ] + } + ] + } + } + }, + "debugging.nodejs.general.editAndContinue": { + "type": "boolean", + "title": "@UnifiedSettings_EditAndContinue;{FE8A8C3D-328A-476D-99F9-2A24B75F8C7F}", + "default": true, + "order": 2, + "migration": { + "custom": { + "mode": "full", + "inputs": [ + { + "store": "VsUserSettingsRegistry", + "path": "NodejsTools\\Options\\General\\EditAndContinue" + } + ], + "map": [ + { + "result": true, + "matches": [ "True" ] + }, + { + "result": false, + "matches": [ "False" ] + } + ] + } + } + } + }, + "categories": { + "debugging.nodejs": { + "title": "@UnifiedSettings_Nodejs;{FE8A8C3D-328A-476D-99F9-2A24B75F8C7F}" + }, + "debugging.nodejs.general": { + "title": "@UnifiedSettings_General;{FE8A8C3D-328A-476D-99F9-2A24B75F8C7F}", + "legacyOptionPageId": "EF677A38-0953-39C2-A228-2FBE8F8F082E" + } + } +} diff --git a/Nodejs/Product/Nodejs/VSPackage.resx b/Nodejs/Product/Nodejs/VSPackage.resx index bb986d846..c7959dd51 100644 --- a/Nodejs/Product/Nodejs/VSPackage.resx +++ b/Nodejs/Product/Nodejs/VSPackage.resx @@ -138,6 +138,21 @@ General + + Node.js + + + General + + + Wait for input when process exits abnormally + + + Wait for input when process exits normally + + + Enable Edit and Continue + Npm diff --git a/Nodejs/Tests/Core/NodejsGeneralOptionsTests.cs b/Nodejs/Tests/Core/NodejsGeneralOptionsTests.cs new file mode 100644 index 000000000..500e01c10 --- /dev/null +++ b/Nodejs/Tests/Core/NodejsGeneralOptionsTests.cs @@ -0,0 +1,219 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; +using Microsoft.NodejsTools.Options; +using Microsoft.VisualStudio.Shell; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Newtonsoft.Json.Linq; + +namespace NodejsTests +{ + [TestClass] + public class NodejsGeneralOptionsTests + { + private const string PackageGuid = "FE8A8C3D-328A-476D-99F9-2A24B75F8C7F"; + + [TestInitialize] + public void InitializeThreadHelper() + { + var contextField = typeof(ThreadHelper).GetField( + "_joinableTaskContextCache", + BindingFlags.NonPublic | BindingFlags.Static); + if (contextField.GetValue(null) == null) + { + _ = System.Windows.Threading.Dispatcher.CurrentDispatcher; + typeof(ThreadHelper) + .GetMethod("SetUIThread", BindingFlags.NonPublic | BindingFlags.Static) + .Invoke(null, null); + contextField.SetValue(null, Activator.CreateInstance(contextField.FieldType)); + } + } + + [TestMethod, Priority(0)] + public void GeneralOptionsUseLegacyDefaults() + { + var page = new TestGeneralOptionsPage(); + + page.LoadSettingsFromStorage(); + + Assert.IsTrue(page.WaitOnAbnormalExit); + Assert.IsFalse(page.WaitOnNormalExit); + Assert.IsTrue(page.EditAndContinue); + } + + [TestMethod, Priority(0)] + public void GeneralOptionsRefreshUnmodifiedValues() + { + var page = new TestGeneralOptionsPage + { + StoredValues = + { + ["WaitOnAbnormalExit"] = false, + ["WaitOnNormalExit"] = true, + ["EditAndContinue"] = false + } + }; + page.LoadSettingsFromStorage(); + + page.StoredValues["WaitOnAbnormalExit"] = true; + page.StoredValues["WaitOnNormalExit"] = false; + page.StoredValues["EditAndContinue"] = true; + page.RefreshSettingsFromStorage(); + + Assert.IsTrue(page.WaitOnAbnormalExit); + Assert.IsFalse(page.WaitOnNormalExit); + Assert.IsTrue(page.EditAndContinue); + } + + [TestMethod, Priority(0)] + public void GeneralOptionsRefreshPreservesUnsavedConsumerValues() + { + var page = new TestGeneralOptionsPage + { + StoredValues = + { + ["WaitOnAbnormalExit"] = true, + ["WaitOnNormalExit"] = true, + ["EditAndContinue"] = false + } + }; + page.LoadSettingsFromStorage(); + + page.WaitOnAbnormalExit = false; + page.StoredValues["WaitOnNormalExit"] = false; + page.StoredValues["EditAndContinue"] = true; + page.RefreshSettingsFromStorage(); + + Assert.IsFalse(page.WaitOnAbnormalExit); + Assert.IsFalse(page.WaitOnNormalExit); + Assert.IsTrue(page.EditAndContinue); + } + + [TestMethod, Priority(0)] + public void GeneralOptionsSaveAllLegacyValues() + { + var page = new TestGeneralOptionsPage + { + WaitOnAbnormalExit = false, + WaitOnNormalExit = true, + EditAndContinue = false + }; + + page.SaveSettingsToStorage(); + + Assert.AreEqual(false, page.SavedValues["WaitOnAbnormalExit"]); + Assert.AreEqual(true, page.SavedValues["WaitOnNormalExit"]); + Assert.AreEqual(false, page.SavedValues["EditAndContinue"]); + } + + [TestMethod, Priority(0)] + public void UnifiedSettingsManifestMatchesLegacyContract() + { + var manifestPath = Path.Combine( + AppDomain.CurrentDomain.BaseDirectory, + "UnifiedSettings", + "NodejsGeneralOptions.registration.json"); + var manifest = JObject.Parse(File.ReadAllText(manifestPath)); + var properties = (JObject)manifest["properties"]; + var categories = (JObject)manifest["categories"]; + + Assert.AreEqual(3, properties.Count); + AssertSetting( + properties, + "debugging.nodejs.general.waitOnAbnormalExit", + "WaitOnAbnormalExit", + true, + "UnifiedSettings_WaitOnAbnormalExit"); + AssertSetting( + properties, + "debugging.nodejs.general.waitOnNormalExit", + "WaitOnNormalExit", + false, + "UnifiedSettings_WaitOnNormalExit"); + AssertSetting( + properties, + "debugging.nodejs.general.editAndContinue", + "EditAndContinue", + true, + "UnifiedSettings_EditAndContinue"); + + Assert.AreEqual(2, categories.Count); + Assert.AreEqual( + "EF677A38-0953-39C2-A228-2FBE8F8F082E", + (string)categories["debugging.nodejs.general"]["legacyOptionPageId"]); + } + + [TestMethod, Priority(0)] + public void UnifiedSettingsPackageRegistrationPointsToManifest() + { + var pkgdef = File.ReadAllText( + Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "UnifiedSettings.pkgdef")); + + StringAssert.Contains(pkgdef, @"SettingsManifests\{" + PackageGuid + "}"); + StringAssert.Contains( + pkgdef, + @"""ManifestPath""=""$PackageFolder$\UnifiedSettings\NodejsGeneralOptions.registration.json"""); + StringAssert.Contains(pkgdef, @"""CacheTag""=qword:"); + } + + private static void AssertSetting( + JObject properties, + string moniker, + string legacyName, + bool defaultValue, + string resourceName) + { + var property = (JObject)properties[moniker]; + Assert.IsNotNull(property, moniker); + Assert.AreEqual("boolean", (string)property["type"], moniker); + Assert.AreEqual(defaultValue, (bool)property["default"], moniker); + Assert.AreEqual( + string.Format("@{0};{{{1}}}", resourceName, PackageGuid), + (string)property["title"], + moniker); + + var migration = (JObject)property["migration"]["custom"]; + Assert.AreEqual("full", (string)migration["mode"], moniker); + Assert.AreEqual(1, ((JArray)migration["inputs"]).Count, moniker); + Assert.AreEqual( + "VsUserSettingsRegistry", + (string)migration["inputs"][0]["store"], + moniker); + Assert.AreEqual( + @"NodejsTools\Options\General\" + legacyName, + (string)migration["inputs"][0]["path"], + moniker); + + var map = (JArray)migration["map"]; + Assert.AreEqual(2, map.Count, moniker); + Assert.AreEqual(true, (bool)map[0]["result"], moniker); + Assert.AreEqual("True", (string)map[0]["matches"][0], moniker); + Assert.AreEqual(false, (bool)map[1]["result"], moniker); + Assert.AreEqual("False", (string)map[1]["matches"][0], moniker); + } + + private sealed class TestGeneralOptionsPage : NodejsGeneralOptionsPage + { + internal IDictionary StoredValues { get; } = + new Dictionary(); + + internal IDictionary SavedValues { get; } = + new Dictionary(); + + internal override bool? LoadBool(string name) + { + return this.StoredValues.TryGetValue(name, out var value) + ? value + : (bool?)null; + } + + internal override void SaveBool(string name, bool value) + { + this.SavedValues[name] = value; + } + } + } +} diff --git a/Nodejs/Tests/Core/NodejsTests.csproj b/Nodejs/Tests/Core/NodejsTests.csproj index 062cd1c7d..50c65488f 100644 --- a/Nodejs/Tests/Core/NodejsTests.csproj +++ b/Nodejs/Tests/Core/NodejsTests.csproj @@ -83,6 +83,7 @@ + @@ -157,6 +158,16 @@ + + + UnifiedSettings.pkgdef + PreserveNewest + + + UnifiedSettings\NodejsGeneralOptions.registration.json + PreserveNewest + + ResXFileCodeGenerator