exported functions block array detection in plugin-transform-for-of

[skeggse]

Bug Report

• I would like to work on a fix!

Current Behavior

A clear and concise description of the behavior.

Input Code

• repl link

const values = [1, 2];

function optimized() {
  // Gets transformed to a standard array-style for loop.
  for (const value of values);
}

export function deoptimized() {
  // Gets transformed to a for loop that supports iterables.
  for (const value of values);
}

Expected behavior/code

A clear and concise description of what you expected to happen (or code).

Exported functions get the same treatment as far as array detection is concerned.

Babel Configuration (babel.config.js, .babelrc, package.json#babel, cli command, .eslintrc)

$ npx babel --plugins @babel/plugin-transform-for-of input.js

Environment

  System:
    OS: Linux 5.6 Arch Linux
  Binaries:
    Node: 10.19.0 - ~/.nvm/versions/node/v10.19.0/bin/node
    npm: 6.13.7 - ~/src/promise-callbacks/node_modules/.bin/npm
  npmPackages:
    @babel/cli: ^7.8.4 => 7.8.4 
    @babel/core: ^7.9.0 => 7.9.0 
    @babel/plugin-transform-runtime: ^7.9.0 => 7.9.0 
    @babel/preset-env: ^7.9.5 => 7.9.5 
    @babel/runtime: ^7.9.2 => 7.9.2 
    eslint: ^6.8.0 => 6.8.0 
    jest: ^25.2.7 => 25.2.7 
    rollup-plugin-babel: ^4.4.0 => 4.4.0 

• Monorepo: no
• How you are using Babel: cli, rollup-plugin-babel, or the online repl

Possible Solution

Additional context/Screenshots
Add any other context about the problem here. If applicable, add screenshots to help explain.

[babel-bot]

Hey @skeggse! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

[nicolo-ribaudo]

This might be a bit hard to fix.

In order to decide when to apply the "array-only" optimization, @babel/plugin-transform-for-of calls the isGenericType("Array") function, which returns true if and only if the inferred type of values is Array<something>.

babel/packages/babel-traverse/src/path/inference/index.js

Lines 114 to 120 in f6c7bf3

	export function isGenericType(genericName: string): boolean {
	const type = this.getTypeAnnotation();
	return (
	t.isGenericTypeAnnotation(type) &&
	t.isIdentifier(type.id, { name: genericName })
	);
	}

The getTypeAnnotation function then calls the inferencer for Identifier (since values is an identifier):

babel/packages/babel-traverse/src/path/inference/inferer-reference.js

Line 4 in f6c7bf3

export default function(node: Object) {

That inferencer then collects all the assignments done before using that variable:

babel/packages/babel-traverse/src/path/inference/inferer-reference.js

Lines 36 to 40 in f6c7bf3

	let constantViolations = getConstantViolationsBefore(
	binding,
	path,
	functionConstantViolations,
	);

The problem is that, since deoptimized is exported, we can't be 100% sure that it's called before the variable definition. Consider this example:

// a.js

import "./b.js";

var values = [1, 2, 3];

export function fn() {
  console.log(values);
}

// b.js

import { fn } from "./a.js";

fn();

If you run a.js, it will log undefined even if in values appears to be defined before fn. For this reason, it's inferred type cannot be Array<number>.

I see three possible solutions (in order of my preference, starting from the best one):

1. If we are trying to inference the value of a let or const variable that is only assigned when declared and never reassigned, we can safely infer its type from the initializer. This is because if we use that variable before that it's initializer runs (like in my previous example), it will throw a TDZ error before actually getting its value which would otherwise be undefined.
2. If the current file doesn't have any import statements, we can be sure that the exported function isn't called by external files before that the current file is fully evaluated. In this case, we can use the same logic for exported function (i.e. deoptimized) and of non-exported function (i.e. optimized).
3. If the inferred type is undefined | Array<something>, we can optimize the for-of loop as if it was an array, because when using undefined it would throw anyway (but with the wrong error message).

[skeggse]

I like all of these solutions! 1 is acceptable to me, as are the rest.

[nicolo-ribaudo]

👍

I'm assigning this issue to you since you ticked "I would like to work on a fix!". If you need any help, feel free to ask either here or in our Slack!