Skip to content
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.

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

migrate @babel/preset-env to babel-preset-modules #24780

Merged
merged 3 commits into from Sep 30, 2019

Conversation

erwinmombay
Copy link
Member

No description provided.

Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Believe we should use NO_TRANSPILE for Closure Compiler output.

build-system/compile/compile.js Outdated Show resolved Hide resolved
build-system/compile/single-pass.js Outdated Show resolved Hide resolved
@@ -36,8 +36,8 @@ exports.transpileTs = function(srcDir, srcFilename) {
const tsEntry = path.join(srcDir, srcFilename).replace(/\.js$/, '.ts');
const tsConfig = ts.convertCompilerOptionsFromJson(
{
'module': 'ES6',
'target': 'ES6',
'module': 'esnext',
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@erwinmombay erwinmombay changed the title migrate library to modern preset migrate @babel/preset-env to babel-preset-modules Sep 27, 2019
@erwinmombay erwinmombay merged commit 587f0b4 into ampproject:master Sep 30, 2019
@erwinmombay erwinmombay deleted the modern-preset branch September 30, 2019 21:55
Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Delayed LGTM with a couple of comments that may or may not need to be addressed.

@@ -256,7 +256,7 @@ function compile(entryModuleFilenames, outputDir, outputFilename, options) {
assume_function_wrapper: true,
// Transpile from ES6 to ES5 if not running with `--esm`
// otherwise transpilation is done by Babel
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this comment need to be updated?

@@ -20,6 +20,7 @@
"dependencies": {
"@ampproject/animations": "0.2.0",
"@ampproject/worker-dom": "0.19.0",
"babel-preset-modules": "0.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should @babel/preset-env have been removed from package.json?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants