♻️ Migrate Style and DOM helpers into core/DOM + type-checking#34681
♻️ Migrate Style and DOM helpers into core/DOM + type-checking#34681rcebulko merged 28 commits intoampproject:mainfrom
Conversation
10606da to
46a6bc4
Compare
|
Hey @erwinmombay! These files were changed: Hey @jridgewell! These files were changed: Hey @alanorozco! These files were changed: Hey @Jiaming-X! These files were changed: Hey @jeffkaufman! These files were changed: Hey @gmajoulet! These files were changed: Hey @processprocess! These files were changed: Hey @t0mg! These files were changed: Hey @mszylkowski! These files were changed: Hey @newmuis! These files were changed: |
3af1d99 to
27a8bb4
Compare
|
@jridgewell building locally to verify, but I suspect the bundle-size diff is all Brotli noise |
|
Because of how I did the move (temporarily had `main:src/dom.js` vs `pr:src/core/dom/index.js`diff --git a/src/dom.js b/src/core/dom/index.js
index ee39dac4e..f766a3dd3 100644
--- a/src/dom.js
+++ b/src/core/dom/index.js
@@ -14,12 +14,9 @@
* limitations under the License.
*/
-import {Deferred} from './core/data-structures/promise';
-import {dev, devAssert} from './log';
-import {dict} from './core/types/object';
-import {includes} from './core/types/string';
-import {matches} from './core/dom/query';
-import {toWin} from './core/window';
+import {dict} from '../types/object';
+import {matches} from './query';
+import {toWin} from '../window';
const HTML_ESCAPE_CHARS = {
'&': '&',
@@ -31,12 +28,6 @@ const HTML_ESCAPE_CHARS = {
};
const HTML_ESCAPE_REGEX = /(&|<|>|"|'|`)/g;
-/** @const {string} */
-export const UPGRADE_TO_CUSTOMELEMENT_PROMISE = '__AMP_UPG_PRM';
-
-/** @const {string} */
-export const UPGRADE_TO_CUSTOMELEMENT_RESOLVER = '__AMP_UPG_RES';
-
/**
* @typedef {{
* bubbles: (boolean|undefined),
@@ -54,17 +45,15 @@ const DEFAULT_CUSTOM_EVENT_OPTIONS = {bubbles: true, cancelable: true};
* @param {!Element} parent
* @param {function(!Element):boolean} checkFunc
* @param {function()} callback
- * @suppress {suspiciousCode}
+ * @suppress {suspiciousCode} due to IS_ESM
*/
export function waitForChild(parent, checkFunc, callback) {
if (checkFunc(parent)) {
callback();
return;
}
- /** @const {!Window} */
const win = toWin(parent.ownerDocument.defaultView);
if (IS_ESM || win.MutationObserver) {
- /** @const {MutationObserver} */
const observer = new win.MutationObserver(() => {
if (checkFunc(parent)) {
observer.disconnect();
@@ -73,7 +62,6 @@ export function waitForChild(parent, checkFunc, callback) {
});
observer.observe(parent, {childList: true});
} else {
- /** @const {number} */
const interval = win.setInterval(() => {
if (checkFunc(parent)) {
win.clearInterval(interval);
@@ -119,9 +107,7 @@ export function waitForBodyOpenPromise(doc) {
* @param {!Element} element
*/
export function removeElement(element) {
- if (element.parentElement) {
- element.parentElement.removeChild(element);
- }
+ element.parentElement?.removeChild(element);
}
/**
@@ -247,7 +233,7 @@ export function rootNodeFor(node) {
/**
* Determines if value is actually a `ShadowRoot` node.
- * @param {HTMLElement} value
+ * @param {?HTMLElement} value
* @return {boolean}
*/
export function isShadowRoot(value) {
@@ -282,7 +268,7 @@ export function getDataParamsFromAttributes(
const computeParamNameFunc = opt_computeParamNameFunc || ((key) => key);
const {dataset} = element;
const params = dict();
- const paramPattern = opt_paramPattern ? opt_paramPattern : /^param(.+)/;
+ const paramPattern = opt_paramPattern || /^param(.+)/;
for (const key in dataset) {
const matches = key.match(paramPattern);
if (matches) {
@@ -348,37 +334,6 @@ export function iterateCursor(iterable, cb) {
}
}
-/**
- * This method wraps around window's open method. It first tries to execute
- * `open` call with the provided target and if it fails, it retries the call
- * with the `_top` target. This is necessary given that in some embedding
- * scenarios, such as iOS' WKWebView, navigation to `_blank` and other targets
- * is blocked by default.
- *
- * @param {!Window} win
- * @param {string} url
- * @param {string} target
- * @param {string=} opt_features
- * @return {?Window}
- */
-export function openWindowDialog(win, url, target, opt_features) {
- // Try first with the specified target. If we're inside the WKWebView or
- // a similar environments, this method is expected to fail by default for
- // all targets except `_top`.
- let res;
- try {
- res = win.open(url, target, opt_features);
- } catch (e) {
- dev().error('DOM', 'Failed to open url on target: ', target, e);
- }
-
- // Then try with `_top` target.
- if (!res && target != '_top' && !includes(opt_features || '', 'noopener')) {
- res = win.open(url, '_top');
- }
- return res;
-}
-
/**
* Whether the element is a script tag with application/json type.
* @param {!Element} element
@@ -387,8 +342,7 @@ export function openWindowDialog(win, url, target, opt_features) {
export function isJsonScriptTag(element) {
return (
element.tagName == 'SCRIPT' &&
- element.hasAttribute('type') &&
- element.getAttribute('type').toUpperCase() == 'APPLICATION/JSON'
+ element.getAttribute('type')?.toUpperCase() == 'APPLICATION/JSON'
);
}
@@ -400,7 +354,7 @@ export function isJsonScriptTag(element) {
export function isJsonLdScriptTag(element) {
return (
element.tagName == 'SCRIPT' &&
- element.getAttribute('type').toUpperCase() == 'APPLICATION/LD+JSON'
+ element.getAttribute('type')?.toUpperCase() == 'APPLICATION/LD+JSON'
);
}
@@ -459,45 +413,6 @@ export function isIframed(win) {
return win.parent && win.parent != win;
}
-/**
- * Determines if this element is an AMP element
- * @param {!Element} element
- * @return {boolean}
- */
-export function isAmpElement(element) {
- const tag = element.tagName;
- // Use prefix to recognize AMP element. This is necessary because stub
- // may not be attached yet.
- return (
- tag.startsWith('AMP-') &&
- // Some "amp-*" elements are not really AMP elements. :smh:
- !(tag == 'AMP-STICKY-AD-TOP-PADDING' || tag == 'AMP-BODY')
- );
-}
-
-/**
- * Return a promise that resolve when an AMP element upgrade from HTMLElement
- * to CustomElement
- * @param {!HTMLElement} element
- * @return {!Promise<!AmpElement>}
- */
-export function whenUpgradedToCustomElement(element) {
- devAssert(isAmpElement(element), 'element is not AmpElement');
- if (element.createdCallback) {
- // Element already is CustomElement;
- return Promise.resolve(/**@type {!AmpElement} */ (element));
- }
- // If Element is still HTMLElement, wait for it to upgrade to customElement
- // Note: use pure string to avoid obfuscation between versions.
- if (!element[UPGRADE_TO_CUSTOMELEMENT_PROMISE]) {
- const deferred = new Deferred();
- element[UPGRADE_TO_CUSTOMELEMENT_PROMISE] = deferred.promise;
- element[UPGRADE_TO_CUSTOMELEMENT_RESOLVER] = deferred.resolve;
- }
-
- return element[UPGRADE_TO_CUSTOMELEMENT_PROMISE];
-}
-
/**
* Returns true if node is not disabled.
*
@@ -604,7 +519,7 @@ export function dispatchCustomEvent(node, name, opt_data, opt_options) {
const event = node.ownerDocument.createEvent('Event');
// Technically .data is not a property of Event.
- /**@type {?}*/ (event).data = data;
+ event.data = data;
const {bubbles, cancelable} = opt_options || DEFAULT_CUSTOM_EVENT_OPTIONS;
event.initEvent(name, bubbles, cancelable); |
|
@jridgewell bumping this |
|
What is the reason for |
AMPElement doesn't exist yet in core, and it's possible this will be called with Bento elements or something else. PausableInterface is the minimal type contract we can specify here. |
Standardize test name
Fix up types and modernize some syntax
Comment tweaks for dom/fullscreen
Fix stubbed test module
…ebook-like-bento-version * 'main' of github.com:ampproject/amphtml: minor updates + fix broken links (ampproject#34840) Add "wrapper": "bento" option to Bento components (ampproject#34838) ♻️ Move src/layout into core to unblock buildDOM for amp-layout (ampproject#34818) ♿ Apply `lang="en"` to relevant snippets in `test/` (ampproject#34768) Bento: Enable `npm` for `amp-video` (ampproject#34822) ✨[story-ads] Introduce new yellow segment progress bar v2 (ampproject#34804) SwG Release (ampproject#34825) 📦 Update build-system devDependencies to v7.14.5 (ampproject#34802) Disable viewport warnings in experiment. (ampproject#34809) Apply lang="en" to examples/ (ampproject#34759) ✨ [Amp story] Scaffold desktop one panel experiment (ampproject#34755) ♻️ Migrate Style and DOM helpers into core/DOM + type-checking (ampproject#34681) 🏗 Don't pull all externs into experiments (ampproject#34800) typechecking: remove pride as not compatible with rest of strategy (ampproject#34787) Fix forbidden terms to unblock `main` (ampproject#34799)
Part of #34096 and #32693
"real" diff: a0f19fb...5e6309c
Moves a bulk of DOM-related modules into core, makes type-checking pass, and modernizes some syntax
Moves the following files:
Resulting structure:
Test files under
test/unit/coremirror this structure