refactor: move the bit-aspect template from core to the core-aspect-env - #10531
Open
davidfirst wants to merge 4 commits into
Open
refactor: move the bit-aspect template from core to the core-aspect-env#10531davidfirst wants to merge 4 commits into
davidfirst wants to merge 4 commits into
Conversation
PR Summary by QodoRefactor: register bit-aspect template via core-aspect-env (not core)
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
Code Review by Qodo
1.
|
…plate-to-env # Conflicts: # pnpm-lock.yaml
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 5ba03fd |
GiladShoham
approved these changes
Jul 27, 2026
|
Code review by qodo was updated up to the latest commit c589e75 |
…plate-to-env # Conflicts: # pnpm-lock.yaml
Comment on lines
+242
to
+266
| createAspect(name = 'my-aspect', options: { path?: string } = {}) { | ||
| const scope = this.workspaceJsonc.getDefaultScope() || this.scopes.remote; | ||
| const scopeWithoutOwner = scope.includes('.') ? scope.split('.')[1] : scope; | ||
| const dir = options.path || path.join(scopeWithoutOwner, name); | ||
| const namePascalCase = name | ||
| .split('-') | ||
| .map((part) => capitalize(part)) | ||
| .join(''); | ||
| this.fs.outputFile( | ||
| path.join(dir, 'index.ts'), | ||
| `import { ${namePascalCase}Aspect } from './${name}.aspect'; | ||
|
|
||
| export type { ${namePascalCase}Main } from './${name}.main.runtime'; | ||
| export default ${namePascalCase}Aspect; | ||
| export { ${namePascalCase}Aspect }; | ||
| ` | ||
| ); | ||
| this.fs.outputFile( | ||
| path.join(dir, `${name}.aspect.ts`), | ||
| `import { Aspect } from '@teambit/harmony'; | ||
|
|
||
| export const ${namePascalCase}Aspect = Aspect.create({ | ||
| id: '${scope}/${name}', | ||
| }); | ||
| ` |
There was a problem hiding this comment.
1. Namespaced aspect scaffolding breaks 🐞 Bug ≡ Correctness
FixtureHelper.createAspect() uses the raw component name to build TypeScript identifiers and filenames, so names containing namespaces (e.g. ui/my-aspect) will produce invalid TS identifiers (contain /) and unintended nested file paths. This makes the new helper incorrect for valid, namespaced component IDs and can cause fixture setup/compile/add steps to fail when such names are used.
Agent Prompt
### Issue description
`createAspect(name)` assumes `name` is a simple kebab-case basename. In Bit, component IDs can include namespaces via `/` (e.g. `ui/my-aspect`). Using the raw `name` in (1) TS identifiers (`namePascalCase`) and (2) filenames like `${name}.aspect.ts` produces invalid TypeScript and wrong nested paths.
### Issue Context
The e2e suite already treats names with `/` as valid component names (e.g. `ui/my-aspect`) via the `bit create` flow; the fixture helper should either support these names or explicitly reject them.
### Fix Focus Areas
- components/legacy/e2e-helper/e2e-fixtures-helper.ts[242-290]
### Suggested fix
- Split the input into `fullName` vs `leafName`:
- `const parts = name.split('/'); const leafName = parts.pop()!; const namespacePath = parts.join('/');`
- Build `namePascalCase` from `leafName` (and sanitize further if needed).
- Generate filenames/imports using `leafName` (e.g. `${leafName}.aspect.ts`, `${leafName}.main.runtime.ts`).
- Keep the component ID passed to Bit as the full name (`i: name`) and keep `Aspect.create({ id: '${scope}/${name}' })` using the full name.
- For `dir`, avoid duplicating the namespace:
- either include `namespacePath` once (e.g. `path.join(scopeWithoutOwner, namespacePath, leafName)`),
- or require callers to pass `options.path` when `name` includes `/` (and throw with a clear error otherwise).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit dff64b1 |
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
2 participants
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.
The `bit-aspect` template is no longer registered by `teambit.harmony/aspect`. It now lives in `teambit.harmony/envs/core-aspect-env` (the env used by the aspects of this repo), which registers it via its `generators()` handler, and components created from it get that env instead of `teambit.harmony/aspect`.