🏗♻️ Consolidate all babel configs into babel.config.js

[rsimha]

The compilation tasks and checks in the amphtml toolchain apply babel transforms to code in a variety of ways. Today, the configs for these transforms are peppered across build-system/, resulting in a hard-to-maintain mess of config code.

#27117 was filed to address this problem, and #27161 took a first stab at fixing this. This PR picks up from there, and does the following:

• Moves all config code out of build-system/compile/build.conf.js and deletes the file
• Implements configs for all babel, babelify, and gulp-babel invocations in build-system/babel-config/*-config.js
• Adds a caller identifier to every invocation site
• Repurposes babel.config.js to return the config that corresponds to the caller

With this PR, all babel configs will live in build-system/babel-config/, while the resulting runtime binaries remain unchanged.

Fixes #27117
Closes #27161

[rsimha]

@jridgewell @erwinmombay @kristoferbaxter babel.config.js has now been fully refactored. However, it was not easy to figure out which portions of build.conf.js overrode / supplemented the original babel.config.js.

I tried reverse engineering this by printing the configurations before / after refactor, but there were too many files for which to inspect the diffs.

It will be significantly easier if you folks took a look at this file and pointed out things that need to be changed.

[rcebulko]

Given how long I spent trying to parse a lot of this code, I'm impressed at how much you were able to consolidate. That said, is there any good way for use to validate these changes to ensure we didn't miss some small thing? For instance, if we were to run each of the build commands/babel configs, would we be able to diff the outputs and verify that we got the exact same results, or are the changes expected to impact the output in some way?

[rsimha]

would we be able to diff the outputs and verify that we got the exact same results

See #27576 (comment) for how we can make sure the same babel transforms are applied. Also, re: code output, the stated goal (in the description) is that nothing should change. We'll eventually get there, after multiple rounds of review and testing.

[kristoferbaxter]

I'm not certain I get this PR yet.

Everything related to Babel is in one place, but the callers are strings that execute other functions, which return static Object literals?

Would it make more sense to make the babel.config.js file lean (contains imports for these callers from a subdirectory inside babel or similar) and delegate the functionality with a prescribed lifecycle?

[rsimha]

Everything related to Babel is in one place, but the callers are strings that execute other functions, which return static Object literals?

No, all the caller objects are merely signatures that help babel.config.js determine where a babel transform invocation originated, and consequently, which config to return. See https://babeljs.io/docs/en/options#caller.

Would it make more sense to make the babel.config.js file lean (contains imports for these callers from a subdirectory inside babel or similar) and delegate the functionality with a prescribed lifecycle?

Having babel.config.js import code from elsewhere is against the original stated goals of the effort to centralize all configurations in one place. For more context, see #27117 and #27161 (review).

[rsimha]

It will be significantly easier if you folks took a look at this file and pointed out things that need to be changed.

@kristoferbaxter Can you take a look at the current WIP babel.config.js and point out what the intended set of transforms are for each use case? (It's really difficult to reverse engineer this stuff because there are hundreds of invocations with different configs across all our use cases). Once I get initial input from you, @jridgewell, and @erwinmombay, I can do some testing to make sure we're generating the exact same compiled output after this refactor.

[jridgewell]

🙌

[kristoferbaxter]

On the nature of a single file versus many, I'm not too strongly positioned however it might be worth considering this structure.

babel.config.js
-- babel-configurations
-- - single-pass.js
-- - dist.js
-- - dist-esm.js
-- - build.js
-- - test.js

Each of these files can export an object for the different lifecycles they support.
{ standalone?: () => {}, preClosure?: () => {}, postClosure?: () => {} }

These lifecycle names and meanings can be defined in a class that all exports used by babel.config.js as ConfigOptions use, and can be enforced via static typing (either TypeScript or Closure types in our case).

Additionally, the names of the "caller" is a nice contract for storing them in a Map inside babel.config.js.

note: this is pseudo-code.

import {singlePassConfiguration} from 'babel-configurations/single-pass';

const configOptions = new Map({
  'single-pass': singlePassConfiguration,
  ...
})

export function execute(caller) {
  if (configOptions.has(caller)) {
    const configuration = configOptions.get(caller);
    ...
  }
}

[kristoferbaxter]

Configs look like they should be accurate for esm and dist afaik.

[kristoferbaxter]

@rsimha – A heads up on an additional transform added today. Will need to add this to the post-closure-babel order.

#27613

[rsimha]

@kristoferbaxter Thanks for the excellent PR feedback! I've incorporated all your comments and marked this PR as ready for review.

Now begins the task of running "before" and "after" builds of all kinds and ironing out config diffs. I'll post an update once this is done, and report if I find any problems.

[kristoferbaxter]

Awesome, I should be able to take another look at this first thing tomorrow.

[amp-owners-bot]

Hey @estherkim, these files were changed:

build-system/tasks/e2e/index.js
build-system/tasks/runtime-test/runtime-test-base.js

[kristoferbaxter]

This is a significant improvement to readability. Thank you for doing this!

[rsimha]

Thanks for the patient reviews, everyone. We are now in a working state with an all-green Travis build.

I did a before/after comparison of the entire dist/ folder output for gulp dist (the variant shipped to stable channel). All minified JS binaries are byte-for-byte identical. I also spot-checked the main AMP binary for a few other variants like gulp dist --fortesting, gulp --compiled, and gulp build and found them to be byte-for-byte identical.

For the remaining callers of babel transforms like gulp dep-check, gulp check-types, gulp dist --single_pass, and gulp dist --esm, I added debug logging to the validate() function in node_modules/@babel/core/lib/config/validation/options.js to print out the final version of the config used by babel / babelify / gulp-babel, and made sure we're using the same config as before in all places.

This PR is now ready to merge.