feat: support multiple yeoman-env versions by ilya-taupeka · Pull Request #541 · SAP/app-studio-toolkit

ilya-taupeka · 2026-06-22T12:30:43Z

update yeoman-environment to 6.1.0 version
update yeoman-generator to 8.2.2 version
add patch loader to fix CJS bundling
add backward compatibility with old generators

- update yeoman-environment to 6.1.0 version - update yeoman-generator to 8.2.2 version - add patch loader to fix CJS bundling - add backward compatibility with old generators

github-actions · 2026-06-22T12:33:28Z

Build Report

Please note:

Files only stay for around 14 days!
This comment will be updated with the data of the last successful build of this PR.

Name	Link
Commit	`9cca7e6`
Logs	https://github.com/SAP/app-studio-toolkit/actions/runs/27952772378
VSIX Files	https://github.com/SAP/app-studio-toolkit/actions/runs/27952772378/artifacts/7792340042

bd82

We might need to go back to the drawing board.

Needed env resolution mode seem fragile and I'm not sure this is resolvable.
Bundling logic of newer yo-env includes actual patches which is also fragile particularly because newer yo-env is a "moving target" (patches will break...).
Old yo-env is also none trivial, but this might be okay as it can be considered a more static version.

To discuss

Dynamic Env Choice logic

is this even feasible in a maintainable / robust manner.
Should we implement an includeList approach to support specific legacy generators instead?

Dynamic Env (legacy / new) Running in VSCode.

One is ESM and one is CJS. This automatically creates none trivial bundling issues.
Should we instead have two Yeoman-UI extensions existing at the same time?

Yeoman-ui-legacy (CJS VSCode Extension)
Yeoman-ui-Modern (ESM VSCode Extension)

That way each *.vsix can handle the bundling in a minimal way most compatiable with its runtime env...

Also the legacy code won't prevent the modern variant from moving forward in terms of tooling / libraries due to CJS backward compatibility.

Please setup a followup meeting to discuss these

bd82 · 2026-06-23T03:42:34Z

+
+    try {
+      const version: string =
+        JSON.parse(fs.readFileSync(pkgJson, "utf8")).version ?? "";


don't use fs.readFileSync use the async versions instead, in this case isLegacyGenerator() is being called from an async method, so doing this is safe and easier on nodejs single threaded event loop.

bd82 · 2026-06-23T03:48:29Z

 import * as path from "path";
-import { existsSync } from "fs";
-import { isWin32, NpmCommand } from "./npm";
+import * as fs from "fs";


use named imports, don't import entire packages.
this makes bundling easier and more minimalist

bd82 · 2026-06-23T03:55:32Z

+//    own node_modules subtree — version major ≤ 5 → legacy
+// 2. Declared dep range: read the generator's own package.json dependencies field —
+//    covers generators that webpack-bundled their deps (no node_modules subfolder)
+// 3. Bundle scan: read the generator's resolved entry file and look for `env.runLoop` —


3 logic is not robust

Bundling with certain configurations can produce multiple entry points.

bundling can produce different levels of minimificaiton of name mangling, so text search on the bundled.js is not safe.

If 3. algo is necessary for the solution it may not be solvable in a dynamic manner and we might need to. go back to include/exclude list of legacy generators.

bd82 · 2026-06-23T04:07:05Z

+  // --- Check 1: private node_modules copy ---
+  const nodeModulesRoots = [
+    path.join(meta.packagePath, "node_modules"),
+    path.join(path.dirname(meta.resolved), "node_modules"),


This second path with meta.resolved does not seem to be a possible (valid) yeoman generator structure.

Did you encounter some legacy generator that required this lookup?

See AI answer:

Short Answer
Probably not in normal Yeoman usage.

Normal Layout
For a typical generator package, dependencies live under the generator package root:

node_modules/generator-foo/ package.json node_modules/yeoman-generator/ generators/app/index.js

So this path is the meaningful one:

path.join(meta.packagePath, "node_modules")

Second Layout
The second path:

path.join(path.dirname(meta.resolved), "node_modules")

would imply something like:

node_modules/generator-foo/ generators/app/ index.js node_modules/yeoman-generator/

That is not a known Yeoman legacy convention. Older Yeoman generators commonly used app/index.js, generators/app/index.js, or lib/generators/app/index.js, but dependencies were still package-level dependencies.

Conclusion
I’d read the second path as defensive or over-broad handling, not a real legacy Yeoman pattern.

A more accurate approach would be Node resolution from the generator file:

require.resolve("yeoman-generator/package.json", { paths: [path.dirname(meta.resolved)], });

That would naturally check nearby node_modules folders and walk upward according to Node’s normal resolution rules.

bd82 · 2026-06-23T04:14:31Z

+// Detects whether a generator requires yeoman-environment v3 (legacy) or v6 (modern)
+//
+// Detection strategy:
+// 1. Private node_modules copy: read yeoman-generator/package.json from the generator's


This makes an assumption on only supporting limited sub-set of valid node_modules structures.

See the full algorithim npm uses for lookup here:

https://docs.npmjs.com/cli/v6/commands/npm-install#algorithm

It is likely better to use import.meta.resolve or require.resolve for full compatability with any current / future node_modules structure, e.g.: dev-spaces recently switched from pnpm to bun for global installs

depending on your runtime env.

Note that these APIs do not look in global paths by default
(but you can probably pass the global paths to search in via their APIs)

bd82 · 2026-06-23T04:56:16Z

  externals: {
    vscode: "commonjs vscode",
+    // yeoman-environment and yeoman-generator are ESM-only packages (type: "module").
+    // They cannot be loaded via CommonJS require() on Node < 22.12 (ERR_REQUIRE_ESM).


do we care about version < 22.12?
The whole 22 major line reaches EOL in ~10 months
and if needed we can control nodejs version on BAS

There seems to be a-lot of complexity here which would likely be reduced significantly if we can run ESM vscode extension more natively

bd82 · 2026-06-23T05:05:58Z

+  "displayName": "yeoman-env-compat",
+  "version": "1.25.0",
+  "private": true,
+  "description": "Thin wrapper that re-exports yeoman-environment v3 and yeoman-generator v4 for backward-compatible legacy generators",


I thought only the old env is needed for legacy generators?

bd82 · 2026-06-23T05:08:03Z

@@ -0,0 +1,33 @@
+{
+  "name": "yeoman-env-compat",


should you encode the versions in the package name?
e.g.:

yeoman-env-v3 -> yeoman-env-compat

e.g.: what happens when you try to create another similar package with yeoman-env v4 in future?

yeoman-env-compat

yeoman-env-compat2

yeoman-env-compat3

bd82 · 2026-06-23T05:08:39Z

@@ -0,0 +1,14 @@
+{


why is a tsconfig needed if there is no *.ts?

bd82 · 2026-06-23T05:13:04Z

@@ -0,0 +1,143 @@
+"use strict";
+
+const path = require("path");


We might be able to trade off *.vsix size to reduce this bundling complexity, basically the *.vsix can include
node_modules but I don't know if its possible for "partial" node_modules (subset).

bd82 · 2026-06-24T10:25:56Z

+  },
+  "dependencies": {
+    "yeoman-environment": "3.19.3",
+    "yeoman-generator": "4.13.0"


does this env bundling require generator inside as well?

ilya-taupeka added 2 commits June 22, 2026 17:02

chore: bump typescript to 5.7.3 and eslint tooling

0419694

chore: update dependencies in yeoman-ui project

c29d089

- update yeoman-environment to 6.1.0 version - update yeoman-generator to 8.2.2 version - add patch loader to fix CJS bundling - add backward compatibility with old generators

ilya-taupeka requested a review from bd82 June 22, 2026 12:30

bd82 reviewed Jun 23, 2026

View reviewed changes

bd82 changed the title ~~chore: update dependencies in yeoman-ui project~~ feat: support multiple yeoman-env versions Jun 24, 2026

bd82 reviewed Jun 24, 2026

View reviewed changes

Uh oh!

Conversation

ilya-taupeka commented Jun 22, 2026

Uh oh!

github-actions Bot commented Jun 22, 2026

Build Report

Uh oh!

bd82 left a comment

Choose a reason for hiding this comment

To discuss

Dynamic Env Choice logic

Dynamic Env (legacy / new) Running in VSCode.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

3 logic is not robust

Uh oh!

Choose a reason for hiding this comment

See AI answer:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants