feat(store): make the generators store independent of an environment - #839
Merged
Merged
Conversation
The store was created by an environment for itself and kept that instance to
create generators from a `createGenerator(environment)` factory and to
instantiate them, so looked up and registered generators could not outlive or
be shared between environments, and nothing could look generators up without
building a whole environment first.
The environment is now passed when it is needed: `importGenerator`,
`instantiate` and `instantiateHelp` of a store meta take it, and fall back to
the one the store was created with, which becomes optional. A generator
exported as a class is the same for every environment; the result of a factory
is kept by environment. The lookup implementation moves from the environment to
`store.lookup()`, the environment passing its `customizeNamespace` and `lookups`
defaults.
An environment accepts a `store` option. The metas of its own store already
default to it and are returned as they were; the metas of a shared store are
bound to the environment asking for them, once per meta, wherever one leaves the
environment - `getGeneratorMeta`, `findMeta`, `getGeneratorsMeta`, `lookup`,
`register` and the `_meta` a generator is instantiated with - so the
`GeneratorMeta` contract of @yeoman/types is unchanged.
Measured with the generators of generator-jhipster (108 registered), medians,
before -> after: new Environment() 0.040 -> 0.038ms, env.lookup 5.17 -> 5.20ms,
getGeneratorMeta of every namespace 0.003 -> 0.004ms, register(stub) x100
2.55 -> 2.56ms, get() cached x100 0.038 -> 0.035ms. With a shared store:
store.lookup 4.87ms, new Environment({ store }) 0.035ms, getGeneratorMeta of
every namespace 0.007ms, getGeneratorsMeta 0.026ms.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
lookupGenerator walked the packages and matched the namespace of each file by itself, a second implementation of what the lookup of the store does. It now asks a store of its own for the generators, which for that grows what the function needs and the lookup did not have: a synchronous `lookupSync`, that `lookup` wraps; a `filter` for the generators to keep, which together with `singleResult` stops at the first one kept; and the `filePath` a generator was found at in the results, as `resolved` is its real path and lookupGenerator returns the path inside `node_modules`, symlink included. The npm paths were looked up even when `packagePaths` was given, where they are not used, and looking them up runs `yarn global dir` and `npm root -g`. They are now only looked up when the packages have to be found. Measured with generator-jhipster as `packagePaths`, medians of 200, before -> after, same results: 'jhipster:app' 81.9 -> 3.4ms, 'jhipster:server' 79.2 -> 3.9ms, every 'jhipster:app' 78.9 -> 4.7ms, packagePath 79.5 -> 3.1ms, a missing generator 78.5 -> 4.8ms. Without `packagePaths` the npm paths are needed either way: 77.4 -> 78.2ms, localOnly 1.41 -> 1.56ms. The environment is unchanged: env.lookup 5.20ms, getGeneratorMeta of every namespace 0.004ms. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
So that generators can be looked up, and a store shared between environments, from outside of the package. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…a of the store The meta of a shared store was bound to an environment as a copy, kept from the first time it was asked for. Anything changing the meta of the store afterwards was then missed by the environments that had already bound it, while the store of an environment itself hands its metas out as they are. generator-jhipster does exactly that in its tests, replacing the `importModule` of mocked generators so their command is still found, and generators were instantiated with a `_meta` that did not have it. The bound meta now has the meta of the store as its prototype, with only the functions that need the environment of its own, and the copies `getGeneratorMeta` and `lookup` return are taken from the meta of the store as it is at that time. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e store With a store of its own an environment returns the record of the store itself, and what is set in it is registered - generator-jhipster registers fake generators in its tests that way. With a shared store a new object with the bound metas was returned, so what was set in it was lost. It is now a view of the record that binds the metas as they are read, which also no longer goes through every meta on each call: 0.026 -> 0.000ms for 108 generators. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The store binds a meta to an environment itself: `get`, `getMeta`, `add`,
`lookup` and `getGeneratorsMeta` take `{ env }` and return metas bound to it,
so the environment no longer keeps its own bound metas.
The meta functions take the environment as options too:
`importGenerator({ env })`, `instantiate(args, options, { env })` and
`instantiateHelp({ env })`.
The Store is marked as experimental.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Store.lookup` and `lookupGenerators` only wrapped their synchronous counterparts, the module lookup being synchronous all the way down: the Promise promised nothing. The Store keeps `lookupSync` only. `Environment.lookup` stays async, as it is public API awaited by its users. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The file path of a lookup is the one globby found, with forward slashes on Windows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation consistently binds shared metadata to the requesting environment and includes focused regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Decouples generator storage from individual environments, enabling safe store sharing.
Changes:
- Adds environment-independent lookup and environment-bound generator metadata.
- Supports injecting and sharing stores across environments.
- Adds comprehensive shared-store and standalone-store tests.
| File | Description |
|---|---|
src/store.ts |
Implements independent lookup, metadata binding, and environment-aware instantiation. |
src/environment-base.ts |
Supports injected stores and binds metadata to each environment. |
src/environment-full.ts |
Returns environment-bound generator metadata. |
src/generator-lookup.ts |
Moves generator lookup registration into Store. |
src/index.ts |
Exports the experimental Store API. |
test/store.ts |
Tests environment-independent store behavior. |
test/shared-store.ts |
Tests sharing one store across environments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of this pull request?
What changes did you make?
Is there anything you'd like reviewers to focus on?