Skip to content

Commit

Permalink
Multi-version: refactor all script-related utils to one module (#33025)
Browse files Browse the repository at this point in the history
* Multi-version: refactor all script-related utils to one module

* remove unused functions

* lints

* fix type

* cleanup

* fix tests

* additional tests for parseExtensionUrl

* lints
  • Loading branch information
Dima Voytenko committed Mar 3, 2021
1 parent b5810d5 commit 81b8b9b
Show file tree
Hide file tree
Showing 14 changed files with 262 additions and 345 deletions.
4 changes: 2 additions & 2 deletions build-system/server/app.js
Expand Up @@ -1390,7 +1390,7 @@ app.get(
const fileName = path.basename(req.path).replace('.max.', '.');
let filePath = 'https://cdn.ampproject.org/v0/' + fileName;
if (mode == 'cdn') {
// This will not be useful until extension-location.js change in prod
// This will not be useful until extension-script.js change in prod
// Require url from cdn
request(filePath, (error, response) => {
if (error) {
Expand Down Expand Up @@ -1446,7 +1446,7 @@ app.get(
const mode = SERVE_MODE;
const fileName = path.basename(req.path);
if (mode == 'cdn') {
// This will not be useful until extension-location.js change in prod
// This will not be useful until extension-script.js change in prod
// Require url from cdn
const filePath = 'https://cdn.ampproject.org/' + fileName;
request(filePath, function (error, response) {
Expand Down
8 changes: 4 additions & 4 deletions build-system/test-configs/dep-check-config.js
Expand Up @@ -325,7 +325,7 @@ exports.rules = [
'src/service/real-time-config/real-time-config-impl.js',
// Parsing extension urls.
'extensions/amp-a4a/0.1/head-validation.js->' +
'src/service/extension-location.js',
'src/service/extension-script.js',
'extensions/amp-video/0.1/amp-video.js->' +
'src/service/video-manager-impl.js',
'extensions/amp-video-iframe/0.1/amp-video-iframe.js->' +
Expand Down Expand Up @@ -405,9 +405,9 @@ exports.rules = [
'src/service/navigation.js',
'extensions/amp-skimlinks/0.1/link-rewriter/link-rewriter-manager.js->' +
'src/service/navigation.js',
// Accessing extension-location.calculateExtensionScriptUrl().
// Accessing extension-script.calculateExtensionScriptUrl().
'extensions/amp-script/0.1/amp-script.js->' +
'src/service/extension-location.js',
'src/service/extension-script.js',
// Origin experiments.
'extensions/amp-experiment/1.0/amp-experiment.js->' +
'src/service/origin-experiments-impl.js',
Expand All @@ -419,7 +419,7 @@ exports.rules = [
'extensions/amp-story-auto-ads/0.1/story-ad-localization.js->src/service/localization.js',
// Accessing calculateScriptBaseUrl() for vendor config URLs
'extensions/amp-analytics/0.1/config.js->' +
'src/service/extension-location.js',
'src/service/extension-script.js',
// Experiment moving Fixed Layer to extension
'extensions/amp-viewer-integration/0.1/amp-viewer-integration.js->' +
'src/service/fixed-layer.js',
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-a4a/0.1/head-validation.js
Expand Up @@ -18,7 +18,7 @@ import {Services} from '../../../src/services';
import {getMode} from '../../../src/mode';
import {includes} from '../../../src/string';
import {map} from '../../../src/utils/object';
import {parseExtensionUrl} from '../../../src/service/extension-location';
import {parseExtensionUrl} from '../../../src/service/extension-script';
import {removeElement, rootNodeFor} from '../../../src/dom';
import {urls} from '../../../src/config';

Expand Down Expand Up @@ -174,7 +174,7 @@ function handleScript(extensions, script) {
(isTesting && includes(src, '/dist/'))
) {
const extensionInfo = parseExtensionUrl(src);
if (EXTENSION_ALLOWLIST[extensionInfo.extensionId]) {
if (extensionInfo && EXTENSION_ALLOWLIST[extensionInfo.extensionId]) {
extensions.push(extensionInfo);
}
}
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-analytics/0.1/config.js
Expand Up @@ -17,7 +17,7 @@
import {DEFAULT_CONFIG} from './default-config';
import {Services} from '../../../src/services';
import {assertHttpsUrl} from '../../../src/url';
import {calculateScriptBaseUrl} from '../../../src/service/extension-location';
import {calculateScriptBaseUrl} from '../../../src/service/extension-script';
import {deepMerge, dict, hasOwn} from '../../../src/utils/object';
import {dev, user, userAssert} from '../../../src/log';
import {getChildJsonConfig} from '../../../src/json';
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-script/0.1/amp-script.js
Expand Up @@ -21,7 +21,7 @@ import {Layout, isLayoutSizeDefined} from '../../../src/layout';
import {Purifier} from '../../../src/purifier/purifier';
import {Services} from '../../../src/services';
import {UserActivationTracker} from './user-activation-tracker';
import {calculateExtensionScriptUrl} from '../../../src/service/extension-location';
import {calculateExtensionScriptUrl} from '../../../src/service/extension-script';
import {cancellation} from '../../../src/error';
import {dev, user, userAssert} from '../../../src/log';
import {dict, map} from '../../../src/utils/object';
Expand Down
50 changes: 1 addition & 49 deletions src/element-service.js
Expand Up @@ -15,6 +15,7 @@
*/

import * as dom from './dom';
import {extensionScriptsInNode} from './service/extension-script';
import {
getAmpdoc,
getService,
Expand All @@ -26,29 +27,6 @@ import {
} from './service';
import {pureUserAssert as userAssert} from './core/assert';

/**
* Returns a promise for a service for the given id and window. Also expects an
* element that has the actual implementation. The promise resolves when the
* implementation loaded. Users should typically wrap this as a special purpose
* function (e.g. Services.viewportForDoc(...)) for type safety and because the
* factory should not be passed around.
* @param {!Window} win
* @param {string} id of the service.
* @param {string} extension Name of the custom extension that provides the
* implementation of this service.
* @param {boolean=} opt_element Whether this service is provided by an element,
* not the extension.
* @return {!Promise<*>}
*/
export function getElementService(win, id, extension, opt_element) {
return getElementServiceIfAvailable(
win,
id,
extension,
opt_element
).then((service) => assertService(service, id, extension));
}

/**
* Same as getElementService but produces null if the given element is not
* actually available on the current page.
Expand Down Expand Up @@ -188,32 +166,6 @@ function assertService(service, id, extension) {
));
}

/**
* Get list of all the extension JS files.
* @param {HTMLHeadElement|Element|ShadowRoot} head
* @return {!Array<string>}
*/
export function extensionScriptsInNode(head) {
// ampdoc.getHeadNode() can return null.
if (!head) {
return [];
}
const scripts = {};
// Note: Some extensions don't have [custom-element] or [custom-template]
// e.g. amp-viewer-integration.
const list = head.querySelectorAll(
'script[custom-element],script[custom-template]'
);
for (let i = 0; i < list.length; i++) {
const script = list[i];
const name =
script.getAttribute('custom-element') ||
script.getAttribute('custom-template');
scripts[name] = true;
}
return Object.keys(scripts);
}

/**
* Waits for body to be present then verifies that an extension script is
* present in head for installation.
Expand Down
17 changes: 5 additions & 12 deletions src/multidoc-manager.js
Expand Up @@ -28,7 +28,7 @@ import {disposeServicesForDoc, getServicePromiseOrNullForDoc} from './service';
import {getMode} from './mode';
import {installStylesForDoc} from './style-installer';
import {isArray, isObject} from './types';
import {parseExtensionUrl} from './service/extension-location';
import {parseExtensionUrl} from './service/extension-script';
import {parseUrlDeprecated} from './url';
import {setStyle} from './style';

Expand Down Expand Up @@ -430,25 +430,18 @@ export class MultidocManager {
dev().fine(TAG, '- src script: ', n);
const src = n.getAttribute('src');
const urlParts = parseExtensionUrl(src);
const isRuntime = !urlParts.extensionId;
// Note: Some extensions don't have [custom-element] or
// [custom-template] e.g. amp-viewer-integration.
const customElement = n.getAttribute('custom-element');
const customTemplate = n.getAttribute('custom-template');
if (isRuntime) {
const extensionId = customElement || customTemplate;
if (!urlParts) {
dev().fine(TAG, '- ignore runtime script: ', src);
} else if (customElement || customTemplate) {
} else if (extensionId) {
// This is an extension.
this.extensions_.installExtensionForDoc(
ampdoc,
customElement || customTemplate,
urlParts.extensionVersion
);
dev().fine(
TAG,
'- load extension: ',
customElement || customTemplate,
' ',
extensionId,
urlParts.extensionVersion
);
if (customElement) {
Expand Down
2 changes: 1 addition & 1 deletion src/service/custom-element-registry.js
Expand Up @@ -17,7 +17,7 @@
import {ElementStub} from '../element-stub';
import {Services} from '../services';
import {createCustomElementClass, stubbedElements} from '../custom-element';
import {extensionScriptsInNode} from '../element-service';
import {extensionScriptsInNode} from './extension-script';
import {reportError} from '../error';
import {pureUserAssert as userAssert} from '../core/assert';

Expand Down
118 changes: 0 additions & 118 deletions src/service/extension-location.js

This file was deleted.

0 comments on commit 81b8b9b

Please sign in to comment.