Add slots and replace/rename functionality. - #40
Conversation
|
❌ There is undocummented changes. Run The following packages have changes but are not documented.
Show changes |
@alloy-js/babel-plugin
@alloy-js/babel-preset
@alloy-js/babel-plugin-jsx-dom-expressions
@alloy-js/core
@alloy-js/csharp
@alloy-js/java
@alloy-js/prettier-plugin-alloy
@alloy-js/typescript
commit: |
Add resolveJavaFQN function
missed checking in index.ts
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
A few issues to address:
javaslots.ts has a debug console.log that should be removed before merge, and transformJavaFqn has dead code (the if (index < parts.length) condition is always true in a .map() callback).
binder.ts has a minor inconsistency where a computed key variable is defined but then name.replace(...) is used directly on the next line instead.
Tests: The rename tests log output to the console but don't assert on it. These tests will pass regardless of actual behavior.
Also noting: this PR currently has merge conflicts and is missing a changeset (per the chronus bot).
| isStatic ? "_" + memberName | ||
| : "." + memberName | ||
| : ""); | ||
| console.log("Resolved Java FQN", fqn); |
There was a problem hiding this comment.
This console.log looks like debug output that shouldn't ship. Remove it before merge.
| return parts | ||
| .map((part, index) => { | ||
| if (index < parts.length) { | ||
| return parts.slice(0, index + 1).join("_"); |
There was a problem hiding this comment.
The condition if (index < parts.length) is always true when iterating with .map() (index goes from 0 to length-1). The else branch is unreachable dead code. You can remove the conditional and just return the parts.slice(0, index + 1).join("_") directly.
Also worth adding a doc comment explaining what this function produces. For input "com.example.service" it outputs "com.com_example.com_example_service" (cumulative underscore-joined prefixes). That behavior isn't obvious from reading the code.
| return waiting.get(key) as Ref<TScope | undefined>; | ||
| } | ||
|
|
||
| const scopeRef = shallowRef<OutputScope | undefined>(undefined); |
There was a problem hiding this comment.
Line 873 defines const key = name.replace(/\./g, "_") but this line uses name.replace(/\./g, "_") again instead of reusing key. Minor inconsistency.
| console.log(tree.contents[0].contents); | ||
| }); | ||
|
|
||
| it("can rename the same thing, last wins", () => { |
There was a problem hiding this comment.
This test (and the "can rename the same thing, last wins" test below it) ends with console.log(...) but has no expect() assertions. The test will always pass regardless of actual output. Consider adding assertions that verify the rename actually took effect in the rendered tree.
3 participants
No description provided.