Don't rely on IDE=VSCode in querystring for behaviour

[DanTup]

Dart-Code/Dart-Code#5970 is a request to change the values pass in the querystring passed to DevTools to include the proper IDE name and not hard-coded values like "VSCode" (which prevents distinguishing VS Code forks using the same extension, like Antigravity or Cursor).

However, there are some places that currently assume "VSCode" here in DevTools that I think require reviewing first (or, we need to pass two values).

1. Survey IDE filters
2. WASM/JS fallback message
3. Property editor documentation link

1. Survey IDE Filters

devtools/packages/devtools_app/lib/src/shared/development_helpers.dart

Lines 229 to 231 in 6046c5b

	// This list is optional and can be used to limit the survey to a specific
	// set of users based on their development environment.
	'devEnvironments': ['Android-Studio', 'IntelliJ-IDEA', 'VSCode', 'CLI'],

I don't know if this is used, but if it is, it would need to cover each of the variations of editors now. I think we could either:

1. Update this comment to mention this is based on DASH__IDE_NAME and include updated examples ("Visual Studio Code", "Antigravity", "Cursor")
2. Pass two values in querystring, DASH__IDE_NAME and also DASH__TOOL (or DASH__PLUGIN_NAME) which are constant across different IDE flavours using the same plugin/extension

2. WASM/JS fallback message

devtools/packages/devtools_app/lib/src/shared/preferences/preferences.dart

Lines 239 to 250 in 6046c5b

	// Do not show the JS fallback notification when embedded in VS Code
	// because we do not expect the WASM build to load successfully by
	// default. This is because cross-origin-isolation is disabled by VS
	// Code. See https://github.com/microsoft/vscode/issues/186614.
	final embeddedInVsCode =
	queryParams.embedMode.embedded && queryParams.ide == 'VSCode';
	if (!embeddedInVsCode) {
	notificationService.push(
	'Something went wrong when trying to load DevTools with WebAssembly. '
	'Falling back to Javascript.',
	);
	}

It controls whether to show a message if the wasm version fails. I'm not sure if this is necessary anymore, as I think there were WASM changes to not require COI? If so, this code could probably just be deleted (and if not, I think won't cause any problems). Needs testing though.

3. Property editor documentation link

devtools/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart

Lines 172 to 179 in 6046c5b

	String? _documentationLink() {
	final queryParams = DevToolsQueryParams.load();
	final isEmbedded = queryParams.embedMode.embedded;
	if (!isEmbedded) return null;
	const uriPrefix = 'https://docs.flutter.dev/tools/';
	const uriHash = '#property-editor';
	return '$uriPrefix${queryParams.ide == 'VSCode' ? 'vs-code' : 'android-studio'}$uriHash';
	}

If IDE is not exactly "VSCode" this will link to the Android Studio docs.

[kenzieschmoll]

I propose we add a helper method isVSCodeVariant or similar that contains the set of possible DASH__IDE_NAME values set by Dart Code.

[DanTup]

I propose we add a helper method isVSCodeVariant or similar that contains the set of possible DASH__IDE_NAME values set by Dart Code.

This set is not known to us. We know of some (VS Code, Cursor, Antigravity, etc.), but new VS Code forks pop up every day and they can provide their own arbitrary strings (there are also various flavours of VS Code, like Insiders, Dev, OSS).

I think if we need these checks, we should pass some additional flag/identifier that is static?

[kenzieschmoll]

I see. Then it sounds like we will need two values. Perhaps we should leave the ide= query param as is then and pass a new key (ideVariant?)

In our analytics, we can either:

1. add another dimension for ideVariant and pass that along with analytics events (no dashboard updates required unless we want to slice by this value)
2. set the ideLaunched value to ideVariant if is is non-null and fallback to the value of the ide query param otherwise (will require updating DevTools dashboards that check for "VSCode")
3. since GA only allows 25 event parameters, we can combine the IDE data into a single value we can parse in our dashboard scripts. Something like ide | ideVariant | ideFeature, as long as this must is less than 100 characters. We'd have to make sure this makes sense for IntelliJ too (will require updating DevTools dashboards to parse this value when the DevTools version is > some version)

Option 3 since it seems like the better long term solution so that we don't bloat our event parameters, but option 1 is the easiest solution today and we could always move to option 3 later if we need to due to event parameter limits.

[DanTup]

I don't know enough about the analytics to have an opinion on how they're recorded/combined, though for consistency to make it clearer what values to expect, what about adding two new QS params to match the new env vars?

• dashTool (which gets the value we put into DASH__TOOL)
• dashIdeName (which gets the value we put into DASH__IDE_NAME)

This would give the front-end the same values that the DevTools server is getting (same-ish names, and same values).

Then treat the existing IDE param as a fallback. Checking that needs to know specifically if it's VS Code would be something like:

var isVsCode = parameters['dashTool'] == 'vscode-plugins' || parameters['ide'] == "VSCode";

Then in some future when all IDEs/plugins we care about are passing the new values, both the IDE plugins and DevTools could drop IDE?

[kenzieschmoll]

This would give the front-end the same values that the DevTools server is getting (same-ish names, and same values).

Where are these getting sent to the DevTools server and what are the values used for in the server?

The plan to use dashTool and dashIdeName SGTM as long as we also update the IntelliJ plugin to pass these query parameters, and as long as we have a plan to clean up the tech debt and remove the ide query parameter at some point.

To summarize, the DevTools query parameters set by the IDE would look like this:

• ide: [DEPRECATE] No changes, continue to include. Remove after some time period (maybe 2 years to follow existing IDE deprecation policy?)
• ideFeature: No changes, continue to include
• dashTool: [NEW] 'vscode-plugins' or 'intellij-plugins'
• dashIdeName: [NEW] 'Visual Studio Code', 'Cursor', 'Antigravity', 'IntelliJ Community', 'Android Studio', etc.

Summary of the required changes as I understand them.

Required changes for Dart-Code extension and IntelliJ plugin:

• Add the dashTool and dashIdeName query parameters to the DevTools URL
• File an issue to remove the ide query parameter in 2 years.

Required changes for DevTools:

• Create a helper / getter isVsCode that returns parameters['dashTool'] == 'vscode-plugins' || parameters['ide'] == "VSCode". Use this for:
  • The WASM / JS Fallback message (see above)
  • The Property editor doc link (see above)
  • Add special handling if devEnvironments contains "VSCode" that checks the new isVsCode getter: https://github.com/flutter/devtools/blob/master/packages/devtools_app/lib/src/shared/managers/survey.dart#L256
• Analytics updates: for consistency, I propose we lean into the dashTool and dashIdeName updates.
  • Deprecate the ideLaunched parameter in the Event.devtoolsEvent constructor: https://github.com/dart-lang/tools/blob/main/pkgs/unified_analytics/lib/src/event.dart#L500
  • Add two new parameters to the Event.devtoolsEvent constructor, dashTool and dashIdeName.
  • Publish the unified_analytics changes and depend on the new package version from DevTools
  • In DevTools where analytics events are constructed, set the values
    • dashTool: parameters['dashTool'] ?? parameters['ide']
    • dashIdeName: parameters['dashIdeName']

Required changes for dashboards:

• Where our dashboards slice by IDE, check for ideLaunched = "VSCode" OR dashTool = "vscode-plugins" OR dashTool = "VSCode" and similarly for checks for "Android Studio" or "IntelliJ".

[DanTup]

Where are these getting sent to the DevTools server and what are the values used for in the server?

Sorry, what I wrote was perhaps misleading. They are provided to the server in environment variables, but the server does not use them. The env vars we discussed in the Dart-Code PR are being set for all processes spawned by Dart-Code (and I believe @helin24 is doing similar for IntelliJ).

I'm hopeful that all of the tools will converge on using those same variables/values so we don't have to manage different things for different tools we spawn, and so it'd be nice if where we do need to provide something other than the env vars, the names/values were still consistent (eg. if you know the values of DASH__TOOL in the env vars, you can assume the same values would be in a querystring value named dashTool).

Your summary all looks good to me, although I think:

Add special handling if devEnvironments contains "VSCode" that checks the new isVsCode getter: https://github.com/flutter/devtools/blob/master/packages/devtools_app/lib/src/shared/managers/survey.dart#L256

... would need to handle IntelliJ too. I would suggest them using the dashTool values instead (either with a mapping from the old IDE values onto the corresponding dashTool values, or we update the explanation the meaning of this to be dashTool values directly - although I don't know if some of this is outside of the DevTools repo?).