Improve babel-helper-define-polyfill-provider resolveSource logic

[zloirock]

Some years ago, I found that babel-helper-define-polyfill-provider couldn't properly determine Number instance properties (in Babel 7, number literals produce NumericLiteral nodes), and templates weren't recognized as strings. I didn't have time to explore the problem further.

Now I'm working on a Babel plugin for core-js@4, and I need better type inference logic for instance properties -)

[nicolo-ribaudo]

Thank you! Can you add also a test case for the ambiguous (a + b).foo?

[nicolo-ribaudo]

Code changes look good to me

[zloirock]

Can you add also a test case for the ambiguous (a + b).foo?

It's already here. Sure, the case that could be determined, but I think it's future work.

[nicolo-ribaudo]

Awesome, thanks again :)

[zloirock]

It seems some more moments. For example, a - b could be bigint. I'll review it again.

[zloirock]

Yes, bigint significantly complicates the inference...

[zloirock]

Done.

[nicolo-ribaudo]

Thank you! I'm going to merge this and release.

We are doing some clean up and dropping old Node.js versions (#250), so if we will then need more releases also for the Babel 7 release line those will take a bit more effort.

[zloirock]

@nicolo-ribaudo I wanted to save compatibility of the plugin with Babel@7 - will it be compatible? Sure, since Node requirement will be bumped - not in the scope of preset-env@7.

[zloirock]

It seems because Removed the CJS build - not completely.

[jackjsj]

Hi @zloirock @nicolo-ribaudo,

I noticed that the changes to resolveSource in #249 may introduce an infinite loop issue. Like this:

Problem

The new version of resolveSource adds recursive calls to itself in several cases:

case "SequenceExpression": {
  return resolveSource(expressions[expressions.length - 1]);
}
case "AssignmentExpression": {
  return resolveSource(...get("right"));
}
case "ConditionalExpression": {
  const consequent = resolveSource(...);
  const alternate = resolveSource(...);
}
case "ParenthesizedExpression":
  return resolveSource(...);
// TypeScript/Flow type wrappers also have recursive calls

However, unlike the resolve function which uses a visited Set to prevent infinite loops:

function resolve(
  path: NodePath,
  resolved: Set<NodePath> = new Set(),  // ✅ Has protection
): NodePath | undefined {
  if (resolved.has(path)) return;
  resolved.add(path);
  // ...
}

The resolveSource function has no such protection mechanism.

How it can cause infinite loop

Since resolveSource calls resolve(obj) internally, and resolve may return the same path or a related node, the recursive calls in the new cases can potentially loop indefinitely. For example, with complex nested expressions or edge cases in AST manipulation.

Suggested fix

Add a visited Set parameter to resolveSource:

export function resolveSource(
  obj: NodePath,
  visited: Set<NodePath> = new Set()
): { id: string | null; placement: "prototype" | "static" | null } {
  if (visited.has(obj)) {
    return { id: null, placement: null };
  }
  visited.add(obj);
  
  // ... existing logic ...
  
  // Pass visited to all recursive calls
  case "SequenceExpression": {
    return resolveSource(expressions[expressions.length - 1], visited);
  }
  // ... etc
}

I encountered this infinite loop issue in production. Would you be able to take a look?

Thanks!

[nicolo-ribaudo]

It seems because Removed the CJS build - not completely.

It will be compatible with Babel 7 on Node.js 20+, which supports require(esm)

[nicolo-ribaudo]

@jackjsj could you open a PR?