feat: support yeoman-environment v3 and v6

[ilya-taupeka]

No description provided.

[github-actions]

Build Report

Please note:

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

Name	Link
Commit	d4c2603
Logs	https://github.com/SAP/app-studio-toolkit/actions/runs/29093159217
VSIX Files	https://github.com/SAP/app-studio-toolkit/actions/runs/29093159217/artifacts/8227663741

[bd82]

lets discuss in person after you review the comments

[bd82]

This looks much better and less complex than initial iterations 👍

I think the main thing to resolve are:

1. Assumption that all loading errors and env error vs possible valid generator error.
2. More edge cases fixtures / tests

[bd82]

Perhaps this logic can be improved, instead of assuming every error is an env error

• Could we identify the specific env errors when loading wrong env?
• Does the current implementation mean we are swallowing and not showing valid errors on generator init?
• We don't need to solve all the various matrix of env's possible errors, only a sub-set because we only have V3/V6 envs right now.

AI Review Comment related to this:

[Medium] v6 generator errors are retried as legacy incompatibility

This fails when a modern v6 generator throws from module import, constructor, or _postConstruct for a real generator error, such as invalid options or missing project state. The broad catch treats every v6 creation failure as a runtime-compatibility signal and calls createLegacyEnvAndGen() with the same generator metadata, so generator initialization can run a second time under v3 and the user may see the fallback failure instead of the original v6 error.

Suggested direction: only fall back for recognized v6/v3 shape or loading incompatibility errors, and rethrow errors that originate from the generator after it has started importing or constructing.

Validation: add a v6 fixture generator whose constructor or _postConstruct increments a counter and throws; createEnvAndGen() should reject with that original error and the counter should be 1, not retry the same generator via v3.

[bd82]

[Low] Let names describe the v6/default routing flow

This comment is long enough that it is describing control flow the code itself could express. The implementation would be easier to maintain if the default-v6 path and fallback-v3 path were visible in names instead of explained in a paragraph.

Suggested direction:

• Log routing generator ${genNamespace} to default yeoman-environment v6.
• Rename local env to v6Env or defaultEnv.
• Rename createLegacyEnvAndGen to createV3EnvAndGen or createLegacyV3EnvAndGen.
• Keep only a short comment for the non-obvious policy, e.g. // v6 is the default runtime; retry with v3 for legacy generator shapes.

Example

this.logger?.debug(
  `routing generator ${genNamespace} to default yeoman-environment v6`
);

try {
  const v6Env: Environment = this.createEnvInstance(
    { sharedOptions: { forwardErrorToEnvironment: true } as any },
    adapter
  );

  v6Env.register(meta.resolved!, {
    namespace: genNamespace,
    packagePath: meta.packagePath,
  });

  const gen: any = await v6Env.create(genNamespace, { options } as any);

  return { env: v6Env, gen };
} catch (error) {
  this.logger?.info(
    `default yeoman-environment v6 could not create ${genNamespace}, falling back to legacy yeoman-environment v3`,
    { error: (error as Error)?.message }
  );
  return this.createLegacyV3EnvAndGen(genNamespace, meta, options, adapter);
}

[bd82]

Do we really need the complexity of lazy loading? how much time does it save?
If we did not do lazy loading, could we also avoid the copy into dist folder step?
basically treat this as a regular package?

• Note the main/exports in package json of legacyV3Env pacakge.json can point to its bundled artifact if neede

[bd82]

this comment exposes too much CI/Build context in the scope of the source code.
If we change the bundling logic in webpack/package.json we might forget to update this comment
to align with new logic.

So, do we really need this duplication?
also see comment below, if we simplify the v3Legacy loading we won't need to comment about it explaining the complexity.

[bd82]

[Low] Prefer names over a line-by-line namespace comment

The doc comment repeats each transformation performed below, so it can easily become stale if the namespace handling changes. The function would be easier to maintain if the intermediate value was named after the Yeoman concept it represents.

e.g.:

// Yeoman namespaces may include a sub-generator suffix, e.g. "generator-foo:app".
export function namespaceToName(namespace: string): string {
  const packageNamespace = namespace.replace(/:.*$/, "");

  if (packageNamespace.startsWith("@")) {
    return packageNamespace.replace("/generator-", "/");
  }

  return packageNamespace.replace(/^generator-/, "");
}

[bd82]

This is none trivial bundling, even if this iteration is might simpler than previous ones.

[Medium]: You could add a tiny smoke test that tries to load the bundled environment to verify it loads
after bundling.

[LOW]: If you believe you can add small smoke tests to also check the env runs properly after bundling
It might have value because it would allow to distinguish between bundling issues in yeoman-env-v3 vs in the whole backend package. But only if this is not high effort...

[bd82]

it looks env-v3 is already exporting the bundle
so why was does it require copy step to use in backend package?

[bd82]

I think if you move the bundle flow into compile (inline or via another script name)
the topological sorting of pnpm -r will handle this package being READY before backend tries to import it.

So no copy would be needed

[bd82]

we have changesets, we don't need to lock to version of other pacakges.
Use the version of the real env-v3: 3.19.3

Maybe even add a tiny script to validate these two versions are aligned

[bd82]

👍

[bd82]

Neutralizing these does not affect runtime in any way?
So they are not needed at runtime inside our *.vsix?