Skip to content

Commit

Permalink
#7255: Remove data-pb-ready attribute, use symbols (#8120)
Browse files Browse the repository at this point in the history
  • Loading branch information
fregante authored Apr 1, 2024
1 parent 3c2587c commit fa97566
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 289 deletions.
1 change: 0 additions & 1 deletion knip.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const knipConfig = {
"static/*",

// App messenger and common storage
"src/contentScript/externalProtocol.ts",
"src/background/messenger/external/api.ts",
"src/store/browserExtensionIdStorage.ts",

Expand Down
8 changes: 0 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@
"nunjucks": "^3.2.4",
"object-hash": "^2.2.0",
"one-event": "^4.2.0",
"one-mutation": "^2.1.0",
"p-defer": "^4.0.0",
"p-memoize": "^7.0.0",
"p-timeout": "^6.1.2",
Expand Down
14 changes: 6 additions & 8 deletions src/contentScript/contentScript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ import "./contentScript.scss";
import { addContentScriptIndicator } from "@/development/visualInjection";
import { uuidv4 } from "@/types/helpers";
import {
isInstalledInThisSession,
setInstalledInThisSession,
setReadyInThisDocument,
unsetReadyInThisDocument,
isContentScriptInstalled,
setContentScriptInstalled,
setContentScriptReady,
} from "@/contentScript/ready";
import { onContextInvalidated } from "webext-events";
import { logPromiseDuration } from "@/utils/promiseUtils";
Expand Down Expand Up @@ -57,7 +56,7 @@ async function initContentScript() {
const urlInfo = top === self ? "" : `in frame ${location.href}`;
const uuid = uuidv4();

if (isInstalledInThisSession()) {
if (isContentScriptInstalled()) {
// Must prevent multiple injection because repeat messenger registration causes message handling errors:
// https://github.com/pixiebrix/webext-messenger/issues/88
// Prior to 1.7.31 we had been using `webext-dynamic-content-scripts` which can inject the same content script
Expand All @@ -84,7 +83,7 @@ async function initContentScript() {

console.debug(`contentScript: importing ${uuid} ${urlInfo}`);

setInstalledInThisSession();
setContentScriptInstalled();

// Keeping the import separate ensures that no side effects are run until this point
const contentScriptPromise = import(
Expand All @@ -96,10 +95,9 @@ async function initContentScript() {

const { init } = await contentScriptPromise;
await logPromiseDuration("contentScript: ready", init());
setReadyInThisDocument(uuid);
setContentScriptReady();

onContextInvalidated.addListener(() => {
unsetReadyInThisDocument(uuid);
console.debug("contentScript: invalidated", uuid);
});

Expand Down
211 changes: 0 additions & 211 deletions src/contentScript/externalProtocol.ts

This file was deleted.

9 changes: 3 additions & 6 deletions src/contentScript/loadActivationEnhancementsCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
loadActivationEnhancements,
TEST_unloadActivationEnhancements,
} from "@/contentScript/loadActivationEnhancementsCore";
import { isReadyInThisDocument } from "@/contentScript/ready";
import { isContentScriptReady } from "@/contentScript/ready";
import { isLinked } from "@/auth/authStorage";
import { array } from "cooky-cutter";
import { MARKETPLACE_URL } from "@/urlConstants";
Expand All @@ -49,10 +49,7 @@ jest.mock("@/auth/authStorage", () => ({

const isLinkedMock = jest.mocked(isLinked);

jest.mock("@/contentScript/ready", () => ({
isReadyInThisDocument: jest.fn(() => true),
}));

jest.mock("@/contentScript/ready");
jest.mock("@/store/extensionsStorage");
jest.mock("@/background/messenger/external/_implementation");
jest.mock("@/sidebar/store");
Expand All @@ -79,7 +76,7 @@ describe("marketplace enhancements", () => {
beforeEach(() => {
document.body.innerHTML = "";
document.body.innerHTML = getDocument(activateButtonsHtml).body.innerHTML;
jest.mocked(isReadyInThisDocument).mockImplementation(() => true);
jest.mocked(isContentScriptReady).mockImplementation(() => true);
getActivatedModIdsMock.mockResolvedValue(new Set());
getActivatingModsMock.mockResolvedValue([]);
});
Expand Down
15 changes: 6 additions & 9 deletions src/contentScript/loadActivationEnhancementsCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { isEmpty, startsWith } from "lodash";
import { DEFAULT_SERVICE_URL, MARKETPLACE_URL } from "@/urlConstants";
import { getActivatedModIds } from "@/store/extensionsStorage";
import { pollUntilTruthy } from "@/utils/promiseUtils";
import { isReadyInThisDocument } from "@/contentScript/ready";
import { isContentScriptReady } from "@/contentScript/ready";
import { getRegistryIdsFromActivateUrlSearchParams } from "@/activation/activationLinkUtils";
import {
type ACTIVATE_EVENT_DETAIL,
Expand Down Expand Up @@ -97,15 +97,12 @@ export async function loadActivationEnhancements(): Promise<void> {
button.addEventListener("click", async (event) => {
event.preventDefault();

const isContentScriptReady = await pollUntilTruthy(
isReadyInThisDocument,
{
maxWaitMillis: 2000,
intervalMillis: 100,
},
);
const isReady = await pollUntilTruthy(isContentScriptReady, {
maxWaitMillis: 2000,
intervalMillis: 100,
});

if (isContentScriptReady) {
if (isReady) {
const detail: ACTIVATE_EVENT_DETAIL = {
// The button href may have changed since the listener was added, extract ids again
activateUrl: button.href,
Expand Down
Loading

0 comments on commit fa97566

Please sign in to comment.