Skip to content

Commit

Permalink
🚀 Remove doc css and base css from ESM build (#26889)
Browse files Browse the repository at this point in the history
* Remove imported base css for valid transformed documents
* Ensure defaultPlugins is always called with a boolean
  • Loading branch information
kristoferbaxter committed Feb 22, 2020
1 parent f674102 commit 99c5984
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 53 deletions.
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"globals": {
"ANALYTICS_VENDOR_SPLIT": "readonly",
"NATIVE_CUSTOM_ELEMENTS_V1": "readonly",
"IS_ESM": "readonly",

"AMP": "readonly",
"context": "readonly",
Expand Down
27 changes: 21 additions & 6 deletions build-system/compile/build.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ const experimentsConstantBackup = require('../global-configs/experiments-const.j
const localPlugin = name =>
require.resolve(`../babel-plugins/babel-plugin-${name}`);

const defaultPlugins = [
const defaultPlugins = isEsmBuild => [
// TODO(alanorozco): Remove `replaceCallArguments` once serving infra is up.
[localPlugin('transform-log-methods'), {replaceCallArguments: false}],
localPlugin('transform-parenthesize-expression'),
localPlugin('is_minified-constant-transformer'),
localPlugin('transform-amp-extension-call'),
localPlugin('transform-html-template'),
localPlugin('transform-version-call'),
getReplacePlugin(),
getReplacePlugin(isEsmBuild),
];

const esmRemovedImports = {
Expand All @@ -41,11 +41,18 @@ const esmRemovedImports = {
'./polyfills/array-includes': ['installArrayIncludes'],
};

// Removable imports that are not needed for valid transformed documents.
const validTransformedRemovableImports = {
'../build/ampshared.css': ['cssText', 'ampSharedCss'],
'../build/ampdoc.css': ['cssText', 'ampDocCss'],
};

/**
* @param {boolean} isEsmBuild a boolean indicating if this build is for ESM output.
* @return {Array<string|Object>} the minify-replace plugin options that can be
* pushed into the babel plugins array
*/
function getReplacePlugin() {
function getReplacePlugin(isEsmBuild) {
/**
* @param {string} identifierName the identifier name to replace
* @param {boolean} value the value to replace with
Expand All @@ -61,7 +68,7 @@ function getReplacePlugin() {
};
}

const replacements = [];
const replacements = [createReplacement('IS_ESM', isEsmBuild)];
const defineFlag = argv.defineExperimentConstant;

// add define flags from arguments
Expand Down Expand Up @@ -121,7 +128,7 @@ function getJsonConfigurationPlugin() {
* @return {!Array<string|!Array<string|!Object>>}
*/
function plugins({isEsmBuild, isForTesting, isSinglePass, isChecktypes}) {
const applied = [...defaultPlugins];
const applied = [...defaultPlugins(isEsmBuild || false)];
// TODO(erwinm): This is temporary until we remove the assert/log removals
// from the java transformation to the babel transformation.
// There is currently a weird interaction where when we do the transform
Expand All @@ -133,7 +140,15 @@ function plugins({isEsmBuild, isForTesting, isSinglePass, isChecktypes}) {
applied.push(localPlugin('transform-amp-asserts'));
}
if (isEsmBuild) {
applied.push(['filter-imports', {imports: esmRemovedImports}]);
applied.push([
'filter-imports',
{
imports: {
...esmRemovedImports,
...validTransformedRemovableImports,
},
},
]);
}
if (isChecktypes) {
applied.push(localPlugin('transform-simple-object-destructure'));
Expand Down
5 changes: 5 additions & 0 deletions build-system/compile/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ function compile(
if (options.include3pDirectories) {
srcs.push('3p/**/*.js', 'ads/**/*.js');
}
// For ESM Builds, exclude ampdoc and ampshared css from inclusion.
// These styles are guaranteed to already be present on elgible documents.
if (options.esmPassCompilation) {
srcs.push('!build/ampdoc.css.js', '!build/ampshared.css.js');
}
// Many files include the polyfills, but we only want to deliver them
// once. Since all files automatically wait for the main binary to load
// this works fine.
Expand Down
5 changes: 4 additions & 1 deletion build-system/tasks/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ const BABELIFY_GLOBAL_TRANSFORM = {
* Plugins used by Babelify while compiling unminified code
*/
const BABELIFY_PLUGINS = {
plugins: [conf.getReplacePlugin(), conf.getJsonConfigurationPlugin()],
plugins: [
conf.getReplacePlugin(argv.esm || false),
conf.getJsonConfigurationPlugin(),
],
};

const hostname = argv.hostname || 'cdn.ampproject.org';
Expand Down
103 changes: 57 additions & 46 deletions src/amp.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,52 @@ import {stubElementsForDoc} from './service/custom-element-registry';
*/
const shouldMainBootstrapRun = !self.IS_AMP_ALT;

/**
* Execute the bootstrap
* @param {!./service/ampdoc-impl.AmpDoc} ampdoc
* @param {!./service/performance-impl.Performance} perf
*/
function bootstrap(ampdoc, perf) {
startupChunk(self.document, function services() {
// Core services.
installRuntimeServices(self);
installAmpdocServices(ampdoc);
// We need the core services (viewer/resources) to start instrumenting
perf.coreServicesAvailable();
maybeTrackImpression(self);
});
startupChunk(self.document, function adoptWindow() {
adoptWithMultidocDeps(self);
});
startupChunk(self.document, function builtins() {
// Builtins.
installBuiltinElements(self);
});
startupChunk(self.document, function stub() {
// Pre-stub already known elements.
stubElementsForDoc(ampdoc);
});
startupChunk(
self.document,
function final() {
installPullToRefreshBlocker(self);
installAutoLightboxExtension(ampdoc);
installStandaloneExtension(ampdoc);
maybeValidate(self);
makeBodyVisible(self.document);
preconnectToOrigin(self.document);
},
/* makes the body visible */ true
);
startupChunk(self.document, function finalTick() {
perf.tick('e_is');
Services.resourcesForDoc(ampdoc).ampInitComplete();
// TODO(erwinm): move invocation of the `flush` method when we have the
// new ticks in place to batch the ticks properly.
perf.flush();
});
}

if (shouldMainBootstrapRun) {
// Store the originalHash as early as possible. Trying to debug:
// https://github.com/ampproject/amphtml/issues/6070
Expand Down Expand Up @@ -106,52 +152,17 @@ if (shouldMainBootstrapRun) {
}
fontStylesheetTimeout(self);
perf.tick('is');
installStylesForDoc(
ampdoc,
ampDocCss + ampSharedCss,
() => {
startupChunk(self.document, function services() {
// Core services.
installRuntimeServices(self);
installAmpdocServices(ampdoc);
// We need the core services (viewer/resources) to start instrumenting
perf.coreServicesAvailable();
maybeTrackImpression(self);
});
startupChunk(self.document, function adoptWindow() {
adoptWithMultidocDeps(self);
});
startupChunk(self.document, function builtins() {
// Builtins.
installBuiltinElements(self);
});
startupChunk(self.document, function stub() {
// Pre-stub already known elements.
stubElementsForDoc(ampdoc);
});
startupChunk(
self.document,
function final() {
installPullToRefreshBlocker(self);
installAutoLightboxExtension(ampdoc);
installStandaloneExtension(ampdoc);
maybeValidate(self);
makeBodyVisible(self.document);
preconnectToOrigin(self.document);
},
/* makes the body visible */ true
);
startupChunk(self.document, function finalTick() {
perf.tick('e_is');
Services.resourcesForDoc(ampdoc).ampInitComplete();
// TODO(erwinm): move invocation of the `flush` method when we have the
// new ticks in place to batch the ticks properly.
perf.flush();
});
},
/* opt_isRuntimeCss */ true,
/* opt_ext */ 'amp-runtime'
);
if (IS_ESM) {
bootstrap(ampdoc, perf);
} else {
installStylesForDoc(
ampdoc,
ampDocCss + ampSharedCss,
() => bootstrap(ampdoc, perf),
/* opt_isRuntimeCss */ true,
/* opt_ext */ 'amp-runtime'
);
}
});

// Output a message to the console and add an attribute to the <html>
Expand Down

0 comments on commit 99c5984

Please sign in to comment.