Fix helper dependencies in babel runtime

[nicolo-ribaudo]

Q	A
Fixed Issues	Fixes #1, Fixes #2
Patch: Bug Fix?	👍
Major: Breaking Change?
Minor: New Feature?
Tests Added/Pass?
Spec Compliancy?
License	MIT
Doc PR
Any Dependency Changes?

I promise this will be the last iteration of helper dependencies 😆

Commit message:

Replace imports in helpers with the dependency.

This commit updates the getDependency callback passed
to helpers.get(). That callback only used to return an
Identifier or MemberExpression which represents the helper;
now it can return both the reference and the code of the
dependency.
Now babel-runtime/scripts/build-dist.js can easily replace
import foo from "foo" with var foo = require("foo").

As a side effect of this change, helpers dependencies are
directly added to the helper ast instead of the file's, making
the call to getHelper inside File#addHelper actually
pure, as requested in the review of the original helper dependencies PR [1]

[1]: #6254 (comment)

This PR extracts the handling of dependencies out of babel-helpers/src/index.js. That file now exports a getDependencies function which returns a list containing the name of the dependencies used by an helper. It's up to the consumer to ensure that they are already available when the helper is included.

This PR fixes a bug regarding the transpilation of imports inside babel-runtime.

Ref: #6366 (comment)

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/5405/

[loganfsmyth]

I think it's helpful to have helpers that test this core-js injection behavior. Why don't we leave the toArray helper and just add the temporalRef one as well.

[loganfsmyth]

It seems like it has gotten pretty complicated in here.

This commit updates the getDependency callback passed to helpers.get(). That callback only used to return an Identifier or MemberExpression which represents the helper; now it can return both the reference and the code of the dependency

This is the primary reason for these changes, is that right?

Having the helpers.get function return the dependencies too seems like it adds a lot of complexity to this logic and I'm not sure what is driving that change. Could you expand on that to help the discussion?

I'm wondering if we'd be better off changing the helper API into two parts, along the lines of a .getGraph(name: string) that would return a list of all the dependencies with the expectation that if you need to insert the dependencies, you'd do

helpers.getGraph(name).forEach(helper => helpers.get(...

That way generating an individual helper can just assume that your getDependency callback will always point at something that has already been inserted.

Thoughts?

[nicolo-ribaudo]

It seems like I have the tendency to make things way more complex than needed 😅
While writing an answer to your comment I realized that there was no reason to make the helpers.get function inject the dependencies in the helper.

[loganfsmyth]

One small comment to fix, but otherwise looks good to me.

[loganfsmyth]

How do you feel about exposing iterables as part of a public API? To me, I'd prefer to stick with an Array to make life easier for consumers still, so I'd vote to wrap this with Array.from()

[nicolo-ribaudo]

I was in doubt because just returing the iterable is faster than doing iterable -> array -> iterable, but probably it can't be noticed since it is only run once per helper (premature optimizations 🚀 🤣)