Skip to content

bugfix dep #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
HoikanChan wants to merge 5 commits into from
Closed

bugfix dep #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4ee2bc
fix(next): array item didn't update
HoikanChan Aug 12, 2025
453855c
fix(next): jsx slice can't render in for
HoikanChan Aug 12, 2025
9932a7e
fix(next): 修复map场景使用optionalCallExpr
HoikanChan Aug 14, 2025
c994f58
chore: fix dependency to caret range
HoikanChan Aug 14, 2025
a4dbed5
chore: update lock
HoikanChan Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions next-packages/compiler/babel-inula-next-core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@
"@babel/plugin-syntax-typescript": "^7.23.3",
"@babel/traverse": "^7.24.1",
"@babel/types": "^7.24.0",
"@openinula/babel-api": "workspace:*",
"@openinula/jsx-view-parser": "workspace:*",
"@openinula/reactivity-parser": "workspace:*",
"@openinula/view-generator": "workspace:*",
"@openinula/error-handler": "workspace:*",
"@openinula/babel-api": "workspace:^",
"@openinula/jsx-view-parser": "workspace:^",
"@openinula/reactivity-parser": "workspace:^",
"@openinula/view-generator": "workspace:^",
"@openinula/error-handler": "workspace:^",
"@types/babel-types": "^7.0.15",
"@types/babel__generator": "^7.6.8",
"@types/babel__traverse": "^7.6.8",
Expand Down
11 changes: 9 additions & 2 deletions next-packages/compiler/babel-inula-next-core/src/sugarPlugins/jsxSlicePlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,17 @@ function transformJSXSlice(path: NodePath<t.JSXElement> | NodePath<t.JSXFragment
// insert the subcomponent
const sliceComp = t.variableDeclaration('const', [t.variableDeclarator(sliceId, sliceCompNode)]);
path.node.extra = { ...path.node.extra, transformJSXSlice: true };
const jsxSliceAlternative =
t.arrowFunctionExpression(
[],
t.callExpression(t.identifier(importMap.createCompNode), [sliceId])
);

if (path.parentPath.isArrowFunctionExpression()) {
// special case: returned by arrow function
const block = t.blockStatement([
sliceComp,
t.returnStatement(t.callExpression(t.identifier(importMap.createCompNode), [sliceId])),
t.returnStatement(jsxSliceAlternative),
]);
path.replaceWith(block);
} else {
Expand All @@ -100,7 +107,7 @@ function transformJSXSlice(path: NodePath<t.JSXElement> | NodePath<t.JSXFragment
throw new Error('Cannot find the statement parent');
}
stmt.insertBefore(sliceComp);
path.replaceWith(t.callExpression(t.identifier(importMap.createCompNode), [sliceId]));
path.replaceWith(jsxSliceAlternative);
}
}

Expand Down
28 changes: 19 additions & 9 deletions next-packages/compiler/babel-inula-next-core/src/sugarPlugins/mapping2ForPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ export default function (api: typeof babel): PluginObj {
Program(program) {
program.traverse({
CallExpression(path: NodePath<t.CallExpression>) {
callExpressionVisitor(path, false);
mapCallVisitor(path, false);
},
OptionalCallExpression(path: NodePath<t.OptionalCallExpression>) {
mapCallVisitor(path, false);
},
});
},
Expand All @@ -45,13 +48,12 @@ export default function (api: typeof babel): PluginObj {
}

/**
* Convert map in JSXExpressionContainer to for visitor
* Convert map call (regular or optional) in JSXExpressionContainer to for visitor
*
* @param path Map call expression path
* @param path Map call expression path (CallExpression or OptionalCallExpression)
* @param inner is inside for tag
*/

function callExpressionVisitor(path: NodePath<t.CallExpression>, inner: boolean): void {
function mapCallVisitor(path: NodePath<t.CallExpression | t.OptionalCallExpression>, inner: boolean): void {
Comment on lines +51 to +56
Copy link

@coderabbitai coderabbitai bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potentially missed cases due to ChainExpression wrappers around optional chains

Optional chaining in Babel can introduce a ChainExpression wrapper around the entire chain. In such cases:

  • For top-level usage in JSX, the OptionalCallExpression’s parent may be ChainExpression, not JSXExpressionContainer.
  • For nested usage inside map callbacks, the immediate parent may be ChainExpression, not ArrowFunctionExpression.

Because the current checks require the immediate parent to be JSXExpressionContainer (outer) or ArrowFunctionExpression (inner), these cases may be skipped and not transformed.

Proposed fix: Loosen the parent checks to tolerate an intermediate ChainExpression, and replace the correct target when doing path.replaceWith(). Here is a robust version of mapCallVisitor illustrating the adjustments (focus: parent checks and replacement target):

function mapCallVisitor(path: NodePath<t.CallExpression | t.OptionalCallExpression>, inner: boolean): void {
  // Helper: climb through a ChainExpression parent if present
  const unwrapChainParent = <T extends Node>(p: NodePath<T>) =>
    p.parentPath && p.parentPath.isChainExpression() ? p.parentPath : p;

  const parent = path.parentPath;
  const parentOrChain = parent && parent.isChainExpression() ? parent.parentPath : parent;

  // Match arrow function map call inside for tag
  if (inner) {
    const isInsideArrow =
      parent?.isArrowFunctionExpression() ||
      (parent?.isChainExpression() && parent.parentPath?.isArrowFunctionExpression());
    if (!isInsideArrow) return;
  }

  // Match map call in JSX expression container
  if (!inner) {
    const isInJsxExpr =
      parent?.isJSXExpressionContainer() ||
      (parent?.isChainExpression() && parent.parentPath?.isJSXExpressionContainer());
    if (!isInJsxExpr) return;
  }

  // Don't convert map call inside any JSXOpeningElement (e.g. attributes)
  if (path.parentPath?.parentPath?.parentPath?.isJSXOpeningElement()) return;

  const callee = path.get('callee');
  // … existing callee handling …

  // Generate for tag ...
  const forElement = /* as in current implementation */;

  // Replace correct target: if callee node is wrapped in a ChainExpression parent, replace that
  const replaceTarget = parent?.isChainExpression() ? parent : path;
  if (replaceTarget.parentPath?.isArrowFunctionExpression()) {
    replaceTarget.replaceWith(forElement);
  } else {
    replaceTarget.parentPath?.replaceWith(forElement);
  }

  // Traverse inner map calls inside the new for
  if (!inner) {
    (replaceTarget.parentPath ?? path.parentPath)?.traverse({
      CallExpression(innerPath: NodePath<t.CallExpression>) {
        mapCallVisitor(innerPath, true);
      },
      OptionalCallExpression(innerPath: NodePath<t.OptionalCallExpression>) {
        mapCallVisitor(innerPath, true);
      },
    });
  }
}

This keeps your current logic but ensures wrapped chains aren’t accidentally skipped.

🤖 Prompt for AI Agents 
In
next-packages/compiler/babel-inula-next-core/src/sugarPlugins/mapping2ForPlugin.ts
around lines 51-56, optional chaining can wrap call expressions in a
ChainExpression so current checks that require the immediate parent to be
JSXExpressionContainer (outer) or ArrowFunctionExpression (inner) will miss
those cases; update the visitor to tolerate an intermediate ChainExpression by
treating parentOrChain = parentPath.isChainExpression() ? parentPath.parentPath
: parentPath (or a helper to unwrap one ChainExpression), use that unwrapped
parent for the JSX/ArrowFunction checks, and when replacing ensure you replace
the ChainExpression node (or its parent) rather than the raw call path (i.e.,
set replaceTarget = parentPath.isChainExpression() ? parentPath : path and then
replace either replaceTarget or replaceTarget.parentPath as appropriate), and
finally traverse inside the newly inserted for element to call mapCallVisitor
for nested CallExpression/OptionalCallExpression occurrences.

//match arrow function map call inside for tag
if (inner && !path.parentPath.isArrowFunctionExpression()) {
return;
Expand All @@ -67,11 +69,16 @@ function callExpressionVisitor(path: NodePath<t.CallExpression>, inner: boolean)
}

const callee = path.get('callee');
if (!callee.isMemberExpression()) {
if (!callee.isMemberExpression() && !callee.isOptionalMemberExpression()) {
return;
}
const object = callee.get('object');
const map = callee.get('property');

// Handle both MemberExpression and OptionalMemberExpression
const objectProperty = callee.get('object');
const object = Array.isArray(objectProperty) ? objectProperty[0] : objectProperty;
const mapProperty = callee.get('property');
const map = Array.isArray(mapProperty) ? mapProperty[0] : mapProperty;

if (!map.isIdentifier()) {
return;
}
Expand Down Expand Up @@ -106,7 +113,10 @@ function callExpressionVisitor(path: NodePath<t.CallExpression>, inner: boolean)
if (!inner) {
path.parentPath.traverse({
CallExpression(path: NodePath<t.CallExpression>) {
callExpressionVisitor(path, true);
mapCallVisitor(path, true);
},
OptionalCallExpression(path: NodePath<t.OptionalCallExpression>) {
mapCallVisitor(path, true);
},
});
}
Expand Down
38 changes: 38 additions & 0 deletions next-packages/compiler/babel-inula-next-core/test/e2e/mappingToFor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,42 @@ describe('mapping2ForPlugin', () => {
}"
`);
});

it('should transform to optional key mapping ', () => {
const code = `
function MyComp({ comment }) {
return (
<div >
{comment.replies?.map(reply => (
<CommentComponent
key={reply.id}
/>
))}
</div>
)
}
`;
const transformedCode = transform(code);
expect(transformedCode).toMatchInlineSnapshot(`
"import { compBuilder as $$compBuilder, createForNode as $$createForNode, createCompNode as $$createCompNode, createHTMLNode as $$createHTMLNode } from "@openinula/next";
function MyComp({
comment
}) {
const $$self = $$compBuilder();
$$self.addProp("comment", $$value => comment = $$value, 1);
return $$self.prepare().init($$createHTMLNode("div", null, $$createForNode(() => comment.replies, () => {
return comment.replies?.map?.(reply => reply.id);
}, ($$n, updateItemFuncArr, reply, $$key, $$i) => {
updateItemFuncArr[$$i] = (newItem, newIdx) => {
reply = newItem;
};
return [$$createCompNode(CommentComponent, {
"key": reply.id
}, $$node => {
$$node.updateProp("key", () => reply.id, [reply?.id], 1);
})];
}, 1)));
}"
`);
});
Comment on lines +73 to +109
Copy link

@coderabbitai coderabbitai bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Great coverage for arr?.map with key mapping; add a couple of edge-case tests

This snapshot thoroughly validates the optional map transformation and key handling. To avoid regressions, consider adding:

  • arr.map?.(cb) form (optional call on the method), e.g., comment.replies.map?.(reply => ).
  • Nested optional mapping inside the callback, e.g., list.map(item => item.children?.map(child => )).
  • Computed property access: comment.replies?.['map']?.(reply => ...).

These ensure both OptionalCallExpression variants and computed properties are covered.

You can check if these cases are already covered anywhere with:

🏁 Script executed:

#!/bin/bash
# 1) Look for existing tests using `map?.(`, nested optional mapping, or computed '["map"]'
rg -n --no-heading "map\?\.\(|\?\.\s*map\(|\?\.\s*\[\s*['\"]map['\"]\s*\]\s*\?\.\s*\(" -A 2 test

# 2) Confirm no handling of `ChainExpression` in the plugin (helps validate the earlier concern)
rg -n --no-heading "ChainExpression" next-packages/compiler/babel-inula-next-core/src/sugarPlugins/mapping2ForPlugin.ts

Length of output: 267

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for literal 'map?.(' occurrences =="
rg -n -F 'map?.(' || true

echo
echo "== Search for literal '?.map(' occurrences =="
rg -n -F '?.map(' || true

echo
echo "== Search for literal '\"map\"' and \"'map'\" (to find computed property uses) =="
rg -n -F '"map"' || true
rg -n -F "'map'" || true

echo
echo "== Locate mapping2ForPlugin.ts =="
mapping_files=$(fd -HI --type f mapping2ForPlugin.ts 2>/dev/null || true)
if [ -z "$mapping_files" ]; then
  mapping_files=$(find . -type f -name mapping2ForPlugin.ts 2>/dev/null || true)
fi
if [ -z "$mapping_files" ]; then
  echo "No mapping2ForPlugin.ts found."
else
  echo "Found files:"
  printf '%s\n' "$mapping_files"
fi

echo
echo "== Show mapping2ForPlugin.ts (first 400 lines) and search for 'ChainExpression' =="
if [ -n "$mapping_files" ]; then
  while IFS= read -r f; do
    echo "---- $f ----"
    sed -n '1,400p' "$f" || true
    echo
    echo "Search for 'ChainExpression' in $f:"
    rg -n "ChainExpression" "$f" || true
    echo
  done <<< "$mapping_files"
fi

echo
echo "== Search for 'ChainExpression' anywhere in repo =="
rg -n "ChainExpression" || true

Length of output: 8138

Good coverage for arr?.map?. — computed-property + nested-callback cases need attention

Short summary: arr?.map?.(...) is already covered in tests, but the mapping plugin only accepts Identifier properties named "map", so computed-access forms (e.g. obj?.['map']?.(...)) will be ignored. Please add tests for the missing edge cases and decide whether to extend the plugin.

Files/locations found

  • Tests already covering optional-call forms:
    • next-packages/compiler/babel-inula-next-core/test/e2e/mappingToFor.test.ts — line ~96: return comment.replies?.map?.(reply => reply.id);
    • next-packages/compiler/babel-inula-next-core/test/generator/view.test.tsx — line ~169: return list?.map?.((item, index) => index);
  • Plugin code that blocks computed-property handling:
    • next-packages/compiler/babel-inula-next-core/src/sugarPlugins/mapping2ForPlugin.ts — contains
      • if (!map.isIdentifier()) { return; }
      • if (map.node.name !== 'map') { return; }
        These checks cause computed property access (e.g. comment.replies?.['map']?.(...)) to be skipped.

Suggested actions

  • Add a test for nested optional mapping inside the callback (e.g. list.map(item => item.children?.map(child => <X key={child.id}/>))).
  • Add a test for computed-property access (comment.replies?.['map']?.(reply => <CommentComponent key={reply.id} />)) to assert current behaviour.
  • If computed-access should be supported, update mapping2ForPlugin.ts to accept computed string-literal/property 'map' (and handle any ChainExpression variants), then add corresponding tests.
🤖 Prompt for AI Agents 
In next-packages/compiler/babel-inula-next-core/test/e2e/mappingToFor.test.ts
around lines 73 to 109, add tests for two missing edge cases: (1) a nested
optional mapping inside the map callback (e.g. list.map(item =>
item.children?.map(child => <X key={child.id}/>))) to ensure inner optional-map
transforms, and (2) a computed-property access form (e.g.
comment.replies?.['map']?.(reply => <CommentComponent key={reply.id} />)) to
capture current behavior. Then decide whether to extend the plugin: if
supporting computed access, update
next-packages/compiler/babel-inula-next-core/src/sugarPlugins/mapping2ForPlugin.ts
to accept computed property access and ChainExpression variants by allowing
MemberExpression with computed string-literal 'map' (and ChainExpression
wrapping) instead of only Identifier named "map", handle extraction of the
property name safely, and add the new tests asserting the transformed output;
otherwise add the computed-access test asserting it remains untransformed.

});
4 changes: 2 additions & 2 deletions next-packages/compiler/babel-inula-next-core/test/e2e/subComponent.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ describe('nested component', () => {
$$setHTMLProp($$node, "value", () => val, [val], 1);
}));
}
let input = $$createCompNode(JSX_input);
return $$self.prepare().init($$createHTMLNode("div", null, $$createExpNode(() => input, () => [input], 2)));
const input = () => $$createCompNode(JSX_input);
return $$self.prepare().init($$createHTMLNode("div", null, $$createExpNode(() => input, () => [], 0)));
}"
`);
});
Expand Down
4 changes: 3 additions & 1 deletion next-packages/compiler/babel-inula-next-core/test/generator/view.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ describe('view generation', () => {
"function Comp() {
const $$self = $$compBuilder();
let list = ['hello', 'world'];
return $$self.prepare().init($$createHTMLNode("div", null, $$createForNode(() => list, () => list.map((item, index) => index), ($$n, updateItemFuncArr, item, $$key, index) => {
return $$self.prepare().init($$createHTMLNode("div", null, $$createForNode(() => list, () => {
return list?.map?.((item, index) => index);
}, ($$n, updateItemFuncArr, item, $$key, index) => {
updateItemFuncArr[index] = (newItem, newIdx) => {
item = newItem;
index = newIdx;
Expand Down
Loading
Loading