vendor node_preamble so the test binary can be used by the Dart SDK

[jakemac53]

Today the Dart SDK does not actually contain all the deps of package:test, which means you can't use the test binary with the SDKs version solve.

It also doesn't allow dependencies from unverified publishers, so vendoring is the easiest option.

This will help unblock potentially moving the SDK to use pub workspaces for resolution instead of a custom package config generator.

[github-actions]

PR Health

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

[devoncarew]

As a drive by comment, if we want to keep the package content as close to the upstream package as possible, this looks good. If however we're forking and want to have less to maintain, you can remove the _minified content above, and the minified and additionalGlobals args here.

[jakemac53]

Yeah I considered that... but I came to the conclusion it would be easiest to just keep it identical. I don't feel strongly either way.

[natebosch]

Do we have any other source where we can get the content for this preamble and not treat it as vendored? @sigmundch do we have any Dart team authored compatibility shim for node?

[natebosch]

This will help unblock potentially moving the SDK to use pub workspaces for resolution instead of a custom package config generator

We should never use this at runtime in the SDK. What if we create a package stub for node_preamble in the SDK repo to satisfy the pub solve? The API can be the same, but return an empty string.

[jakemac53]

I would be fine with just putting a stub in the SDK

[jakemac53]

Closing in favor of https://dart-review.googlesource.com/c/sdk/+/399221

[sigmundch]

I prefer the idea of the stub too. Also, I worry vendoring this way may not be proper. Embedding third-party code requires following special rules (e.g. always be in a third-party folder at the top of the repo).

There isn't a version of this in the SDK already, but one could be created. The contents of the package were largely based on the existing d8 and jsshell preambles (e.g. computeCurrentScript). It just wasn't added in the SDK because it wasn't needed there.

[sigmundch]

Two other options:

• not make this a dependency of package:test, but either a side dependency (e.g. not embed the node runner with the default package:test, but provide it as an additional functionality on a separate package).
• Since the SDK doesn't use node, we could also process the DEPS in the SDK to remove the unused dependencies somehow and ignore it if we can. This is practically what we do with copybara in g3, but I fear it may be harder to do with depot_tools.

[jakemac53]

not make this a dependency of package:test, but either a side dependency (e.g. not embed the node runner with the default package:test, but provide it as an additional functionality on a separate package).

This would be great but isn't feasible to do right now. Maybe some day if we decide to invest in making the test package more pluggable.

• Since the SDK doesn't use node, we could also process the DEPS in the SDK to remove the unused dependencies somehow and ignore it if we can. This is practically what we do with copybara in g3, but I fear it may be harder to do with depot_tools.

Patch files are a nightmare to maintain, they cause constant headaches and should imo be avoided wherever possible, especially as long term solutions.