Skip to content

Commit

Permalink
🏗 Output distinct bento-*.js (#36174)
Browse files Browse the repository at this point in the history
Before this change, output `bento-*.js` files are identical as `amp-*.js`, and not as they should be:

1. They should only work on Bento standalone mode, not AMP mode
2. They should not attempt to use AMP runtime features.

Bento components directories include both `amp-*.js` and `base-element.js`. The former exports an AMP extension, while the latter exports a component-specific `BaseElement` that does not attempt to use AMP runtime features. 

This change generates a `bento-*.js` source file that uses the `BaseElement` implementation without AMP usage.
  • Loading branch information
alanorozco committed Oct 14, 2021
1 parent bf4c1b7 commit 272d342
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 133 deletions.
54 changes: 27 additions & 27 deletions build-system/compile/bundles.config.extensions.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -199,7 +199,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -233,7 +233,7 @@
"latestVersion": "0.1",
"options": {
"hasCss": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -299,7 +299,7 @@
"latestVersion": "0.1",
"options": {
"npm": true,
"wrapper": "bento",
"bento": true,
"hasCss": true
}
},
Expand All @@ -314,7 +314,7 @@
"latestVersion": "0.1",
"options": {
"npm": true,
"wrapper": "bento",
"bento": true,
"hasCss": true
}
},
Expand Down Expand Up @@ -351,7 +351,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -374,7 +374,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -407,7 +407,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -481,7 +481,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -548,7 +548,7 @@
"amp-inline-gallery-pagination"
],
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -571,7 +571,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -614,7 +614,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -632,7 +632,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -678,7 +678,7 @@
"latestVersion": "0.1",
"options": {
"hasCss": true,
"wrapper": "bento",
"bento": true,
"npm": true
}
},
Expand Down Expand Up @@ -827,7 +827,7 @@
"version": "1.0",
"latestVersion": "1.0",
"options": {
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -863,7 +863,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -892,7 +892,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -927,7 +927,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -942,7 +942,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -1088,7 +1088,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -1127,7 +1127,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -1154,7 +1154,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -1177,7 +1177,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -1200,7 +1200,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -1227,7 +1227,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand Down Expand Up @@ -1265,7 +1265,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
},
{
Expand All @@ -1285,7 +1285,7 @@
"options": {
"hasCss": true,
"npm": true,
"wrapper": "bento"
"bento": true
}
}
]
69 changes: 0 additions & 69 deletions build-system/compile/compile-wrappers.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
const {VERSION} = require('./internal-version');

const removeWhitespace = (str) => str.replace(/\s+/g, '');

// If there is a sync JS error during initial load,
// at least try to unhide the body.
// If "AMP" is already an object then that means another runtime has already
Expand Down Expand Up @@ -83,71 +81,4 @@ function extensionPayload(name, version, latest, isModule, loadPriority) {
);
}

/**
* Anonymous function to load a Bento extension's payload (p).
*
* The provided AMP.registerElement function has the same signature as
* `Extensions.addElement` on extensions-impl.js. In order:
* - `name` (n): the name of the custom element tag
* - `Ctor` (c): the constructor function (class) for the custom element
* - `style` (s): optional string to install CSS for the custom element
* @see {@link bento}
* TODO(alanorozco): It would be cleaner if we used a "bento-foo" literal for
* the tag name instead of replacing the prefix in "amp-foo".
* TODO(alanorozco): `/amp.js` is relevant only for unminified builds. We add
* this only so that tests can pass during CI. Eventually, we'll create separate
* binaries and the script check should not be present at all.
*/
const bentoLoaderFn = removeWhitespace(`
function (payload) {
self.AMP
? self.AMP.push(payload)
: document.head.querySelector('script[src$="v0.js"],script[src$="v0.mjs"],script[src$="/amp.mjs"],script[src$="/amp.js"]')
? (self.AMP = [payload])
: payload.f({
registerElement: function (n, b, s) {
if (s)
document.head.appendChild(
document.createElement("style")
).textContent = s;
customElements.define(
n.replace(/^amp-/, 'bento-'),
b.CustomElement(b)
);
},
});
}
`);

/**
* Wraps to load an extension's payload (p) as a Bento component.
*
* It tries to use AMP's loading mechanism (`(self.AMP = self.AMP || []).push`)
* when detecting the runtime either by a global, or the presence of a `script`
* tag.
*
* On Bento documents, the extension's function (f) is executed immediately.
* In this case, a barebones `AMP.registerElement` is also provided.
* It uses a CustomElement implementation provided by the extension class
* itself, and installs extension-specific CSS as soon as possible.
* @param {string} name
* @param {string} version
* @param {boolean} latest
* @param {boolean=} isModule
* @param {ExtensionLoadPriorityDef=} loadPriority
* @return {string}
*/
function bento(name, version, latest, isModule, loadPriority) {
const payload = extensionPayload(
name,
version,
latest,
isModule,
loadPriority
);
return `(${bentoLoaderFn})(${payload});`;
}

exports.bento = bento;

exports.none = '<%= contents %>';
2 changes: 2 additions & 0 deletions build-system/tasks/clean.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const ROOT_DIR = path.resolve(__dirname, '../../');
* @return {Promise<void>}
*/
async function clean() {
// TODO(https://go.amp.dev/issue/36354): We can simplify this list
const pathsToDelete = [
// Local cache directories
// Keep this list in sync with .gitignore, .eslintignore, and .prettierignore
Expand All @@ -36,6 +37,7 @@ async function clean() {
'dist.tools',
'export',
'examples/storybook',
'extensions/**/build',
'extensions/**/dist',
'release',
'result-reports',
Expand Down

0 comments on commit 272d342

Please sign in to comment.