From bc03cd6fc08e7250b2ecdd3453295676c5b9ee11 Mon Sep 17 00:00:00 2001 From: Graham Langford <30706330+grahamlangford@users.noreply.github.com> Date: Wed, 24 Apr 2024 16:21:59 -0500 Subject: [PATCH 1/5] add check that the filtering works (#8334) --- .../tests/regressions/formFlicker.spec.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/end-to-end-tests/tests/regressions/formFlicker.spec.ts b/end-to-end-tests/tests/regressions/formFlicker.spec.ts index 9de5f116ea..7ff18b261b 100644 --- a/end-to-end-tests/tests/regressions/formFlicker.spec.ts +++ b/end-to-end-tests/tests/regressions/formFlicker.spec.ts @@ -46,12 +46,21 @@ test.describe("forms flickering due to components unexpectedly unmounting/remoun const sideBarPage = await getSidebarPage(page, extensionId); + await expect(sideBarPage.getByTestId("container").nth(1)).toContainText( + "Welcome to the Snippet Manager!", + ); + const input = sideBarPage.getByPlaceholder("Search snippets"); await input.click(); // Add delay to give time for the the input to lose focus - await input.pressSequentially("abc", { delay: 100 }); + await input.pressSequentially("lor", { delay: 100 }); // Validate that the input value was set correctly - await expect(input).toHaveValue("abc"); + await expect(input).toHaveValue("lor"); + + // Verify that the input value filtered the list + await expect(sideBarPage.getByTestId("container").nth(1)).toContainText( + "Lorem ipsum dolor sit amet, ", + ); }); }); From dba6fdde697d5ce9b2eec9a2572b5a2df71bcd8c Mon Sep 17 00:00:00 2001 From: Ben Loe Date: Wed, 24 Apr 2024 17:29:17 -0400 Subject: [PATCH 2/5] #8326 - Fix page editor warning indicator when using a brick with the implicit PixieBrix api integration (#8339) * don't filter out pb vars from var analysis * fix types * add pixiebrix integration to the context vars test * cleanup --------- Co-authored-by: Ben Loe --- .../analysisVisitors/varAnalysis/varAnalysis.test.ts | 11 +++++++++-- .../analysisVisitors/varAnalysis/varAnalysis.ts | 7 +------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/analysis/analysisVisitors/varAnalysis/varAnalysis.test.ts b/src/analysis/analysisVisitors/varAnalysis/varAnalysis.test.ts index 222c8e98ef..80c30b5d4d 100644 --- a/src/analysis/analysisVisitors/varAnalysis/varAnalysis.test.ts +++ b/src/analysis/analysisVisitors/varAnalysis/varAnalysis.test.ts @@ -56,6 +56,7 @@ import { CustomFormRenderer } from "@/bricks/renderers/customForm"; import { toExpression } from "@/utils/expressionUtils"; import IdentityTransformer from "@/bricks/transformers/IdentityTransformer"; import { createNewConfiguredBrick } from "@/pageEditor/exampleBrickConfigs"; +import pixiebrixIntegrationDependencyFactory from "@/integrations/util/pixiebrixIntegrationDependencyFactory"; jest.mocked(services.locate).mockResolvedValue( sanitizedIntegrationConfigFactory({ @@ -134,13 +135,14 @@ describe("Collecting available vars", () => { const extension = formStateFactory( { - // Let this extension have an integration dependency + // Test both the PixieBrix api integration and a sample third-party service integrationDependencies: [ integrationDependencyFactory({ integrationId: validateRegistryId("@test/service"), - outputKey: validateOutputKey("pixiebrix"), + outputKey: validateOutputKey("testService"), configId: uuidSequence, }), + pixiebrixIntegrationDependencyFactory(), ], optionsArgs: { foo: "bar", @@ -164,6 +166,11 @@ describe("Collecting available vars", () => { expect(foundationKnownVars.isVariableDefined("@options.foo")).toBeTrue(); + expect( + foundationKnownVars.isVariableDefined( + "@testService.__service.serviceId", + ), + ).toBeTrue(); expect( foundationKnownVars.isVariableDefined("@pixiebrix.__service.serviceId"), ).toBeTrue(); diff --git a/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts b/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts index 73a19ab873..2782264d2d 100644 --- a/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts +++ b/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts @@ -52,7 +52,6 @@ import { MOD_VARIABLE_REFERENCE } from "@/runtime/extendModVariableContext"; import { joinPathParts } from "@/utils/formUtils"; import makeIntegrationsContextFromDependencies from "@/integrations/util/makeIntegrationsContextFromDependencies"; import { getOutputReference, isOutputKey } from "@/runtime/runtimeTypes"; -import { PIXIEBRIX_INTEGRATION_ID } from "@/integrations/constants"; export const INVALID_VARIABLE_GENERIC_MESSAGE = "Invalid variable name"; @@ -114,13 +113,9 @@ async function setIntegrationDependencyVars( { integrationDependencies = [] }: ModComponentFormState, contextVars: VarMap, ): Promise { - // We don't want to make the pixiebrix api integration available as a variable - const nonPbDependencies = integrationDependencies.filter( - (dependency) => dependency.integrationId !== PIXIEBRIX_INTEGRATION_ID, - ); // Loop through all the dependencies, so we can set the source for each dependency variable properly await Promise.all( - nonPbDependencies.map(async (integrationDependency) => { + integrationDependencies.map(async (integrationDependency) => { const serviceContext = await makeIntegrationsContextFromDependencies([ integrationDependency, ]); From 0535e019ef2dad7a136d0600cd973fca9e4f57ee Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Wed, 24 Apr 2024 19:12:35 -0400 Subject: [PATCH 3/5] Improve guideline documentation for business errors (#8321) --- src/errors/businessErrors.ts | 37 +++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/errors/businessErrors.ts b/src/errors/businessErrors.ts index 47d6655575..841f0dee6e 100644 --- a/src/errors/businessErrors.ts +++ b/src/errors/businessErrors.ts @@ -27,16 +27,40 @@ import { JQUERY_INVALID_SELECTOR_ERROR } from "@/errors/knownErrorMessages"; */ /** - * Base class for Errors arising from business logic in the brick, not the PixieBrix application/extension itself. + * Base class for Errors arising from user-defined logic/inputs, not PixieBrix itself. * - * Used for blame analysis for reporting and alerting. + * Where possible, use a more specific subclass of BusinessError. Some subclasses have an enriched error view in + * the mod log viewer. + * + * "Business" Errors vs. "Application" Errors: + * - Application errors (subclasses of Error) indicate a bug or failure in PixieBrix itself, which must be addressed + * by the PixieBrix team + * - Business errors indicate a problem with user-defined content or 3rd party services. They must be addressed + * by the customer + * - Business errors are not reported to Datadog, but are reported to the PixieBrix error telemetry service. See + * recordError and reportToErrorService. + * + * Throw a BusinessError (or a BusinessError subclass) to indicate: + * - A logic error in a package definition + * - A logic error in a brick configuration (i.e., a mod definition, or custom brick definition) + * - A runtime error due to user-provided values (e.g., bad configuration options) + * - A failed 3rd-party API call + * + * Use an Application error (i.e., Error subclass) to indicate: + * - A bug in PixieBrix itself + * - A failed API call to a PixieBrix service (e.g., fetching packages) + * + * Other guidelines: + * - Throw an application error for assertions that should never fail, e.g., that the function caller should have + * checked the precondition before calling a function. This guideline applies even if the condition is due to + * package definition/user input, because it represents a bug in our code. */ export class BusinessError extends Error { override name = "BusinessError"; } /** - * Error that a registry definition is invalid + * Error that a registry package definition is invalid. */ export class InvalidDefinitionError extends BusinessError { override name = "InvalidDefinitionError"; @@ -120,8 +144,11 @@ export class InvalidSelectorError extends BusinessError { } /** - * An error indicating an invalid input was provided to a brick. Used for checks that cannot be performed as part - * of JSONSchema input validation + * An error indicating an invalid input was provided to a brick. Used for runtime checks that cannot be performed as + * part of JSONSchema input validation. + * + * Throwing PropError instead of BusinessError allows the Page Editor to show the error on the associated field + * in the brick configuration UI. * * @see InputValidationError */ From 9d0a15e9e696c377fe50ef819198915c6bab5de6 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Thu, 25 Apr 2024 10:37:37 -0400 Subject: [PATCH 4/5] #8228: run show sidebar in top-level frame (#8299) --- .../tests/runtime/sidebarController.spec.ts | 78 +++++++++++++++++++ .../messenger/strict/registration.ts | 6 +- src/contentScript/sidebarController.tsx | 68 +++++++++++----- src/utils/sidePanelUtils.ts | 12 ++- 4 files changed, 139 insertions(+), 25 deletions(-) create mode 100644 end-to-end-tests/tests/runtime/sidebarController.spec.ts diff --git a/end-to-end-tests/tests/runtime/sidebarController.spec.ts b/end-to-end-tests/tests/runtime/sidebarController.spec.ts new file mode 100644 index 0000000000..445e140627 --- /dev/null +++ b/end-to-end-tests/tests/runtime/sidebarController.spec.ts @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2024 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { test, expect } from "../../fixtures/extensionBase"; +import { ActivateModPage } from "../../pageObjects/extensionConsole/modsPage"; +// @ts-expect-error -- https://youtrack.jetbrains.com/issue/AQUA-711/Provide-a-run-configuration-for-Playwright-tests-in-specs-with-fixture-imports-only +import { type Page, test as base } from "@playwright/test"; +import { getSidebarPage, isSidebarOpen } from "../../utils"; +import { MV } from "../../env"; + +test.describe("sidebar controller", () => { + test("can open sidebar immediately from iframe without focus dialog", async ({ + page, + extensionId, + }) => { + const modId = "@pixies/test/frame-sidebar-actions"; + + const modActivationPage = new ActivateModPage(page, extensionId, modId); + await modActivationPage.goto(); + await modActivationPage.clickActivateAndWaitForModsPageRedirect(); + + await page.goto("/frames-builder.html"); + + const frame = page.frameLocator("iframe"); + await frame.getByRole("link", { name: "Show Sidebar Immediately" }).click(); + + // Don't use getSidebarPage because it automatically clicks the MV3 focus dialog. + await expect(() => { + expect(isSidebarOpen(page, extensionId)).toBe(false); + }).toPass({ timeout: 5000 }); + }); + + test("shows focus dialog in top-level frame", async ({ + page, + extensionId, + }) => { + test.skip(MV === "2", "This test is only relevant for MV3"); + + const modId = "@pixies/test/frame-sidebar-actions"; + + const modActivationPage = new ActivateModPage(page, extensionId, modId); + await modActivationPage.goto(); + await modActivationPage.clickActivateAndWaitForModsPageRedirect(); + + await page.goto("/frames-builder.html"); + + const frame = page.frameLocator("iframe"); + + // Mod waits 7.5 seconds before running Show Sidebar brick to ensure the user gesture dialog is shown + await frame.getByRole("link", { name: "Show Sidebar after Wait" }).click(); + // eslint-disable-next-line playwright/no-wait-for-timeout -- match wait in the mod + await page.waitForTimeout(8000); + + // Focus dialog should be visible in the top-level frame + await expect(page.getByRole("button", { name: "OK" })).toBeVisible(); + + // The focus dialog should not be shown in the iframe. Check after checking the top-level frame + // because it's a positive check for the dialog being shown. + await expect(frame.getByRole("button", { name: "OK" })).not.toBeVisible(); + + // Will error if page/frame not available + await getSidebarPage(page, extensionId); + }); +}); diff --git a/src/contentScript/messenger/strict/registration.ts b/src/contentScript/messenger/strict/registration.ts index f43af5265c..a79b979423 100644 --- a/src/contentScript/messenger/strict/registration.ts +++ b/src/contentScript/messenger/strict/registration.ts @@ -19,7 +19,7 @@ import { hideMv2SidebarInTopFrame, - showSidebar, + showSidebarInTopFrame, sidebarWasLoaded, updateSidebar, removeExtensions as removeSidebars, @@ -54,7 +54,7 @@ declare global { FORM_CANCEL: typeof cancelForm; UPDATE_SIDEBAR: typeof updateSidebar; SIDEBAR_WAS_LOADED: typeof sidebarWasLoaded; - SHOW_SIDEBAR: typeof showSidebar; + SHOW_SIDEBAR: typeof showSidebarInTopFrame; HIDE_SIDEBAR: typeof hideMv2SidebarInTopFrame; REMOVE_SIDEBARS: typeof removeSidebars; HANDLE_MENU_ACTION: typeof handleMenuAction; @@ -84,7 +84,7 @@ export default function registerMessenger(): void { FORM_CANCEL: cancelForm, UPDATE_SIDEBAR: updateSidebar, SIDEBAR_WAS_LOADED: sidebarWasLoaded, - SHOW_SIDEBAR: showSidebar, + SHOW_SIDEBAR: showSidebarInTopFrame, HIDE_SIDEBAR: hideMv2SidebarInTopFrame, REMOVE_SIDEBARS: removeSidebars, HANDLE_MENU_ACTION: handleMenuAction, diff --git a/src/contentScript/sidebarController.tsx b/src/contentScript/sidebarController.tsx index 76fd19aa87..eae83f83ef 100644 --- a/src/contentScript/sidebarController.tsx +++ b/src/contentScript/sidebarController.tsx @@ -19,10 +19,12 @@ import reportEvent from "@/telemetry/reportEvent"; import { Events } from "@/telemetry/events"; import { expectContext } from "@/utils/expectContext"; import sidebarInThisTab from "@/sidebar/messenger/api"; +import * as contentScriptApi from "@/contentScript/messenger/strict/api"; import { isEmpty, throttle } from "lodash"; import { signalFromEvent } from "abort-utils"; import { SimpleEventTarget } from "@/utils/SimpleEventTarget"; import * as sidebarMv2 from "@/contentScript/sidebarDomControllerLite"; +import { getSidebarElement } from "@/contentScript/sidebarDomControllerLite"; import { type Except } from "type-fest"; import { type RunArgs, RunReason } from "@/types/runtimeTypes"; import { type UUID } from "@/types/stringTypes"; @@ -30,8 +32,8 @@ import { type RegistryId } from "@/types/registryTypes"; import { type ModComponentRef } from "@/types/modComponentTypes"; import type { ActivatePanelOptions, - ModActivationPanelEntry, FormPanelEntry, + ModActivationPanelEntry, PanelEntry, PanelPayload, TemporaryPanelEntry, @@ -41,18 +43,29 @@ import { getFormPanelSidebarEntries } from "@/platform/forms/formController"; import { memoizeUntilSettled } from "@/utils/promiseUtils"; import { getTimedSequence } from "@/types/helpers"; import { isMV3 } from "@/mv3/api"; -import { getErrorMessage } from "@/errors/errorHelpers"; import { focusCaptureDialog } from "@/contentScript/focusCaptureDialog"; import { isLoadedInIframe } from "@/utils/iframeUtils"; import { showMySidePanel } from "@/background/messenger/strict/api"; -import { getSidebarElement } from "@/contentScript/sidebarDomControllerLite"; import focusController from "@/utils/focusController"; import selectionController from "@/utils/selectionController"; -import { messenger } from "webext-messenger"; -import { getSidebarTargetForCurrentTab } from "@/utils/sidePanelUtils"; +import { getTopLevelFrame, messenger } from "webext-messenger"; +import { + getSidebarTargetForCurrentTab, + isUserGestureRequiredError, +} from "@/utils/sidePanelUtils"; const HIDE_SIDEBAR_EVENT_NAME = "pixiebrix:hideSidebar"; +/** + * Event listeners triggered when the sidebar shows and is ready to receive messages. + */ +export const sidebarShowEvents = new SimpleEventTarget(); + +// eslint-disable-next-line local-rules/persistBackgroundData -- Unused there +const panels: PanelEntry[] = []; + +let modActivationPanelEntry: ModActivationPanelEntry | null = null; + /* * Only one check at a time * Cannot throttle because subsequent checks need to be able to be made immediately @@ -88,30 +101,37 @@ const pingSidebar = memoizeUntilSettled( }, 1000) as () => Promise, ); -/** - * Event listeners triggered when the sidebar shows and is ready to receive messages. - */ -export const sidebarShowEvents = new SimpleEventTarget(); - export function sidebarWasLoaded(): void { sidebarShowEvents.emit({ reason: RunReason.MANUAL }); } -// eslint-disable-next-line local-rules/persistBackgroundData -- Unused there -const panels: PanelEntry[] = []; - -let modActivationPanelEntry: ModActivationPanelEntry | null = null; - /** - * Attach the sidebar to the page if it's not already attached. Then re-renders all panels. + * Content script handler for showing the sidebar in the top-level frame. Regular callers should call + * showSidebar instead, which handles calls from iframes. + * + * - Resolves when the sidebar is initialized (responds to a ping) + * - Shows focusCaptureDialog if a user gesture is required + * + * @see showSidebar + * @see pingSidebar + * @throws Error if the sidebar ping fails or does not respond in time */ -export async function showSidebar(): Promise { +// Don't memoizeUntilSettled this method. focusCaptureDialog is memoized which prevents this method from showing +// the focus dialog from multiple times. By allowing multiple concurrent calls to showSidebarInTopFrame, +// a subsequent call might succeed, which will then automatically close the focusCaptureDialog (via it's abort signal) +export async function showSidebarInTopFrame() { reportEvent(Events.SIDEBAR_SHOW); + + if (isLoadedInIframe()) { + console.warn("showSidebarInTopFrame should not be called in an iframe"); + } + + // Defensively handle accidental calls from iframes if (isMV3() || isLoadedInIframe()) { try { await showMySidePanel(); } catch (error) { - if (!getErrorMessage(error).includes("user gesture")) { + if (!isUserGestureRequiredError(error)) { throw error; } @@ -137,6 +157,18 @@ export async function showSidebar(): Promise { } } +/** + * Attach the sidebar to the page if it's not already attached. Safe to call from any frame. Resolves when the + * sidebar is initialized. + * @see showSidebarInTopFrame + */ +export async function showSidebar(): Promise { + // Could consider explicitly calling showSidebarInTopFrame directly if we're already in the top frame. + // But the messenger will already handle that case automatically. + const topLevelFrame = await getTopLevelFrame(); + await contentScriptApi.showSidebar(topLevelFrame); +} + /** * Force-show the panel for the given extension id * @param extensionId the extension UUID diff --git a/src/utils/sidePanelUtils.ts b/src/utils/sidePanelUtils.ts index 63b35dfb63..d16d7024f6 100644 --- a/src/utils/sidePanelUtils.ts +++ b/src/utils/sidePanelUtils.ts @@ -24,6 +24,13 @@ import { type PageTarget, messenger, getThisFrame } from "webext-messenger"; import { isContentScript } from "webext-detect-page"; import { showSidebar } from "@/contentScript/messenger/strict/api"; +/** + * Returns true if an error showing sidebar is due to a missing user gesture. + */ +export function isUserGestureRequiredError(error: unknown): boolean { + return getErrorMessage(error).includes("user gesture"); +} + export async function openSidePanel(tabId: number): Promise { if (isBrowserSidebar()) { console.warn( @@ -60,10 +67,7 @@ async function openSidePanelMv3(tabId: number): Promise { // it's still part of a user gesture. // If it's not, it will throw an error *even if the side panel is already open*. // The following code silences that error iff the side panel is already open. - if ( - getErrorMessage(error).includes("user gesture") && - (await isSidePanelOpen(tabId)) - ) { + if (isUserGestureRequiredError(error) && (await isSidePanelOpen(tabId))) { // The `openSidePanel` call was not required in the first place, the error can be silenced // TODO: After switching to MV3, verify whether we drop that `openSidePanel` call return; From 1b2a1d9ed28fed43933c336a279ba68cef510d3a Mon Sep 17 00:00:00 2001 From: Graham Langford <30706330+grahamlangford@users.noreply.github.com> Date: Thu, 25 Apr 2024 12:29:26 -0500 Subject: [PATCH 5/5] #8343: handle 4 digit extension version numbers (#8346) * update semver helpers to handle 4 digit versions * fix deployment check * make 4th digit larger to reflect actual usage * replace direct access with getExtensionVersion() * fix broken tests * fix manifest for connectPage * add 4th digit for all builds * manifest tests * add release version test * revert version change * fix package-lock version * properly fix package.json version * rename validateSemVerString -> normalizeSemVerString * add documentation for getExtensionVersion * test bad values * js docs for normalizeSemVerString --- scripts/__snapshots__/manifest.test.js.snap | 12 +- scripts/manifest.mjs | 24 +-- scripts/manifest.test.js | 98 ++++++--- src/background/backgroundPlatform.ts | 7 +- src/background/deploymentUpdater.test.ts | 8 +- src/background/deploymentUpdater.ts | 2 +- src/background/installer.ts | 9 +- src/background/messenger/external/api.ts | 7 +- .../restrictUnauthenticatedUrlAccess.test.ts | 1 + src/background/telemetry.ts | 13 +- src/contentScript/contentScriptPlatform.ts | 7 +- src/data/service/errorService.ts | 8 +- src/extensionConsole/pages/UpdateBanner.tsx | 3 +- .../ConvertToRecipeModalBody.tsx | 4 +- .../pages/settings/useDiagnostics.ts | 8 +- src/extensionPages/extensionPagePlatform.ts | 7 +- src/pageEditor/panes/save/saveHelpers.test.ts | 8 +- src/pageEditor/sidebar/ModListItem.test.tsx | 4 +- .../sidebar/modals/CreateModModal.tsx | 6 +- src/pageEditor/starterBricks/base.ts | 4 +- src/platform/platformContext.ts | 4 +- src/registry/packageRegistry.test.ts | 4 +- src/runtime/pipelineTests/component.test.ts | 4 +- src/testUtils/factories/brickFactories.ts | 4 +- .../factories/deploymentFactories.ts | 4 +- src/testUtils/factories/metadataFactory.ts | 4 +- src/testUtils/platformMock.ts | 4 +- src/testUtils/testAfterEnv.ts | 3 +- src/tsconfig.strictNullChecks.json | 1 + src/types/helpers.test.ts | 197 ++++++++++++++++++ src/types/helpers.ts | 35 +++- src/utils/deploymentUtils.test.ts | 10 +- src/utils/deploymentUtils.ts | 4 +- src/utils/extensionUtils.ts | 21 +- src/utils/objToYaml.test.ts | 6 +- 35 files changed, 415 insertions(+), 130 deletions(-) create mode 100644 src/types/helpers.test.ts diff --git a/scripts/__snapshots__/manifest.test.js.snap b/scripts/__snapshots__/manifest.test.js.snap index 98bae6d7bd..8b1e8ea04e 100644 --- a/scripts/__snapshots__/manifest.test.js.snap +++ b/scripts/__snapshots__/manifest.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`customizeManifest beta 1`] = ` +exports[`customizeManifest release builds beta 1`] = ` { "action": { "default_icon": { @@ -124,6 +124,8 @@ exports[`customizeManifest beta 1`] = ` "storage": { "managed_schema": "managedStorageSchema.json", }, + "version": "1.8.13.3000", + "version_name": "1.8.13", "web_accessible_resources": [ { "matches": [ @@ -149,7 +151,7 @@ exports[`customizeManifest beta 1`] = ` } `; -exports[`customizeManifest mv2 1`] = ` +exports[`customizeManifest release builds mv2 1`] = ` { "author": "PixieBrix, Inc.", "background": { @@ -264,6 +266,8 @@ exports[`customizeManifest mv2 1`] = ` "storage": { "managed_schema": "managedStorageSchema.json", }, + "version": "1.8.13.3000", + "version_name": "1.8.13", "web_accessible_resources": [ "css/*", "bundles/*", @@ -282,7 +286,7 @@ exports[`customizeManifest mv2 1`] = ` } `; -exports[`customizeManifest mv3 1`] = ` +exports[`customizeManifest release builds mv3 1`] = ` { "action": { "default_icon": { @@ -406,6 +410,8 @@ exports[`customizeManifest mv3 1`] = ` "storage": { "managed_schema": "managedStorageSchema.json", }, + "version": "1.8.13.3000", + "version_name": "1.8.13", "web_accessible_resources": [ { "matches": [ diff --git a/scripts/manifest.mjs b/scripts/manifest.mjs index c8c641e550..462b4bc68c 100644 --- a/scripts/manifest.mjs +++ b/scripts/manifest.mjs @@ -19,10 +19,11 @@ import Policy from "csp-parse"; import { normalizeManifestPermissions } from "webext-permissions"; import { excludeDuplicatePatterns } from "webext-patterns"; -function getVersion(env, isBetaListing) { +function getVersion(env) { const stageMap = { alpha: 1000, beta: 2000, + release: 3000, }; // `manifest.json` only supports numbers in the version, so use the semver @@ -32,19 +33,16 @@ function getVersion(env, isBetaListing) { ); const { version, stage, stageNumber } = match.groups; - // Add 4th digit for alpha/beta release builds. Used to update the extension BETA listing in the Chrome Web Store. - if (isBetaListing) { - if (stage && stageNumber) { - // Ex: 1.8.13-alpha.1 -> 1.8.13.1001 - // Ex: 1.8.13-beta.55 -> 1.8.13.2055 - return `${version}.${stageMap[stage] + Number(stageNumber)}`; - } - - // Ex: 1.8.13.3000 -- Ensures that the release build version number is greater than the alpha/beta build version numbers - return `${version}.3000`; + // Add 4th digit for differentiating alpha/beta/stable release builds. + // Used primarily to update the extension BETA listing in the Chrome Web Store. + if (stage && stageNumber) { + // Ex: 1.8.13-alpha.1 -> 1.8.13.1001 + // Ex: 1.8.13-beta.55 -> 1.8.13.2055 + return `${version}.${stageMap[stage] + Number(stageNumber)}`; } - return version; + // Ex: 1.8.13.3000 -- Ensures that the release build version number is greater than the alpha/beta build version numbers + return `${version}.${stageMap.release}`; } function getVersionName(env, isProduction) { @@ -146,7 +144,7 @@ function addInternalUrlsToContentScripts(manifest, internal) { function customizeManifest(manifestV2, options = {}) { const { isProduction, manifestVersion, env = {}, isBeta } = options; const manifest = structuredClone(manifestV2); - manifest.version = getVersion(env, isBeta); + manifest.version = getVersion(env); manifest.version_name = getVersionName(env, isProduction); if (!isProduction) { diff --git a/scripts/manifest.test.js b/scripts/manifest.test.js index 3d71e281e1..b61212e83c 100644 --- a/scripts/manifest.test.js +++ b/scripts/manifest.test.js @@ -15,9 +15,6 @@ * along with this program. If not, see . */ -/* eslint-disable @shopify/jest/no-snapshots -- We want to specifically commit the entire customized manifest as a snapshot */ -/* eslint-disable no-restricted-imports -- Aliases don't work outside built files */ - import { omit } from "lodash"; import manifest from "../src/manifest.json"; import { loadEnv } from "./env.mjs"; @@ -25,35 +22,76 @@ import customizeManifest from "./manifest.mjs"; loadEnv(); -const cleanCustomize = (...args) => - omit(customizeManifest(...args), ["version", "version_name", "key"]); +const cleanCustomize = (...args) => omit(customizeManifest(...args), ["key"]); describe("customizeManifest", () => { - test("mv2", () => { - expect( - cleanCustomize(manifest, { - env: process.env, - isProduction: true, - }), - ).toMatchSnapshot(); - }); - test("mv3", () => { - expect( - cleanCustomize(manifest, { - env: process.env, - isProduction: true, - manifestVersion: 3, - }), - ).toMatchSnapshot(); + describe("release builds", () => { + test("mv2", () => { + expect( + cleanCustomize(manifest, { + // eslint-disable-next-line camelcase -- auto-inserted + env: { ...process.env, npm_package_version: "1.8.13" }, + isProduction: true, + }), + ).toMatchSnapshot(); + }); + + test("mv3", () => { + expect( + cleanCustomize(manifest, { + // eslint-disable-next-line camelcase -- auto-inserted + env: { ...process.env, npm_package_version: "1.8.13" }, + isProduction: true, + manifestVersion: 3, + }), + ).toMatchSnapshot(); + }); + + test("beta", () => { + expect( + cleanCustomize(manifest, { + // eslint-disable-next-line camelcase -- auto-inserted + env: { ...process.env, npm_package_version: "1.8.13" }, + isProduction: true, + manifestVersion: 3, + isBeta: true, + }), + ).toMatchSnapshot(); + }); }); - test("beta", () => { - expect( - cleanCustomize(manifest, { - env: process.env, - isProduction: true, - manifestVersion: 3, - isBeta: true, - }), - ).toMatchSnapshot(); + + describe("four digit versioning", () => { + test("alpha version", () => { + expect( + cleanCustomize(manifest, { + // eslint-disable-next-line camelcase -- auto-inserted + env: { ...process.env, npm_package_version: "1.8.13-alpha.123" }, + isProduction: true, + manifestVersion: 3, + }), + ).toContainEntry(["version", "1.8.13.1123"]); + }); + + test("beta version", () => { + expect( + cleanCustomize(manifest, { + // eslint-disable-next-line camelcase -- auto-inserted + env: { ...process.env, npm_package_version: "1.8.13-beta.123" }, + isProduction: true, + manifestVersion: 3, + }), + ).toContainEntry(["version", "1.8.13.2123"]); + }); + + test("release version", () => { + expect( + cleanCustomize(manifest, { + // eslint-disable-next-line camelcase -- auto-inserted + env: { ...process.env, npm_package_version: "1.8.13" }, + isProduction: true, + manifestVersion: 3, + }), + ).toContainEntry(["version", "1.8.13.3000"]); + }); }); }); diff --git a/src/background/backgroundPlatform.ts b/src/background/backgroundPlatform.ts index 2ec60d2f57..f828fdf19a 100644 --- a/src/background/backgroundPlatform.ts +++ b/src/background/backgroundPlatform.ts @@ -23,8 +23,8 @@ import type { NetworkRequestConfig } from "@/types/networkTypes"; import type { RemoteResponse } from "@/types/contract"; import { performConfiguredRequest } from "@/background/requests"; import BackgroundLogger from "@/telemetry/BackgroundLogger"; -import { validateSemVerString } from "@/types/helpers"; import { PlatformBase } from "@/platform/platformBase"; +import { getExtensionVersion } from "@/utils/extensionUtils"; /** * Background platform implementation. Currently, just makes API requests. @@ -40,10 +40,7 @@ class BackgroundPlatform extends PlatformBase { }); constructor() { - super( - "background", - validateSemVerString(browser.runtime.getManifest().version), - ); + super("background", getExtensionVersion()); } override get logger(): PlatformProtocol["logger"] { diff --git a/src/background/deploymentUpdater.test.ts b/src/background/deploymentUpdater.test.ts index 1d5fcff8a4..8252856bac 100644 --- a/src/background/deploymentUpdater.test.ts +++ b/src/background/deploymentUpdater.test.ts @@ -19,7 +19,7 @@ import { getModComponentState, saveModComponentState, } from "@/store/extensionsStorage"; -import { uuidv4, validateSemVerString } from "@/types/helpers"; +import { uuidv4, normalizeSemVerString } from "@/types/helpers"; import { appApiMock } from "@/testUtils/appApiMock"; import { omit } from "lodash"; import { syncDeployments } from "@/background/deploymentUpdater"; @@ -68,8 +68,8 @@ jest.mock("@/store/settings/settingsStorage"); jest.mock("@/hooks/useRefreshRegistries"); jest.mock("@/utils/extensionUtils", () => ({ + ...jest.requireActual("@/utils/extensionUtils"), forEachTab: jest.fn(), - getExtensionVersion: () => browser.runtime.getManifest().version, })); // Override manual mock to support `expect` assertions @@ -380,7 +380,7 @@ describe("syncDeployments", () => { _recipe: { id: deployment.package.package_id, name: deployment.package.name, - version: validateSemVerString("0.0.1"), + version: normalizeSemVerString("0.0.1"), updated_at: deployment.updated_at, sharing: sharingDefinitionFactory(), }, @@ -438,7 +438,7 @@ describe("syncDeployments", () => { _recipe: { id: deployment.package.package_id, name: deployment.package.name, - version: validateSemVerString("0.0.1"), + version: normalizeSemVerString("0.0.1"), updated_at: deployment.updated_at, sharing: sharingDefinitionFactory(), }, diff --git a/src/background/deploymentUpdater.ts b/src/background/deploymentUpdater.ts index 6c953e76e2..df0d574922 100644 --- a/src/background/deploymentUpdater.ts +++ b/src/background/deploymentUpdater.ts @@ -608,7 +608,7 @@ async function activateDeploymentsInBackground({ ); // Version to report to the server. - const { version: extensionVersionString } = browser.runtime.getManifest(); + const extensionVersionString = getExtensionVersion(); const extensionVersion = parseSemVer(extensionVersionString); const deploymentsByActivationMethod = await Promise.all( diff --git a/src/background/installer.ts b/src/background/installer.ts index a5829b561b..14123f958a 100644 --- a/src/background/installer.ts +++ b/src/background/installer.ts @@ -36,7 +36,10 @@ import { import { Events } from "@/telemetry/events"; import { DEFAULT_SERVICE_URL, UNINSTALL_URL } from "@/urlConstants"; import { CONTROL_ROOM_TOKEN_INTEGRATION_ID } from "@/integrations/constants"; -import { getExtensionConsoleUrl } from "@/utils/extensionUtils"; +import { + getExtensionConsoleUrl, + getExtensionVersion, +} from "@/utils/extensionUtils"; import { oncePerSession } from "@/mv3/SessionStorage"; import { resetFeatureFlagsCache } from "@/auth/featureFlagStorage"; @@ -223,7 +226,7 @@ export async function showInstallPage({ // https://developer.chrome.com/docs/extensions/reference/runtime/#event-onInstalled // https://developer.chrome.com/docs/extensions/reference/runtime/#type-OnInstalledReason console.debug("onInstalled", { reason, previousVersion }); - const { version } = browser.runtime.getManifest(); + const version = getExtensionVersion(); if (reason === "install") { void recordEvent({ @@ -309,7 +312,7 @@ export function getAvailableVersion(): typeof _availableVersion { */ export function isUpdateAvailable(): boolean { const available = getAvailableVersion(); - const installed = browser.runtime.getManifest().version; + const installed = getExtensionVersion(); return ( Boolean(available) && installed !== available && gt(available, installed) ); diff --git a/src/background/messenger/external/api.ts b/src/background/messenger/external/api.ts index 83801cd14f..6b519a90c8 100644 --- a/src/background/messenger/external/api.ts +++ b/src/background/messenger/external/api.ts @@ -23,9 +23,14 @@ import { _liftBackground as liftExternal } from "@/background/externalProtocol"; import * as local from "@/background/messenger/external/_implementation"; import { readPartnerAuthData } from "@/auth/authStorage"; +import { getExtensionVersion } from "@/utils/extensionUtils"; export const connectPage = liftExternal("CONNECT_PAGE", async () => - browser.runtime.getManifest(), + // Ensure the version we send to the app is a valid semver. + ({ + ...browser.runtime.getManifest(), + version: getExtensionVersion(), + }), ); export const setExtensionAuth = liftExternal( diff --git a/src/background/restrictUnauthenticatedUrlAccess.test.ts b/src/background/restrictUnauthenticatedUrlAccess.test.ts index aca607ff0b..d8d317d39e 100644 --- a/src/background/restrictUnauthenticatedUrlAccess.test.ts +++ b/src/background/restrictUnauthenticatedUrlAccess.test.ts @@ -36,6 +36,7 @@ jest.mock("@/auth/authStorage", () => ({ })); jest.mock("@/utils/extensionUtils", () => ({ + ...jest.requireActual("@/utils/extensionUtils"), forEachTab: jest.fn(), })); diff --git a/src/background/telemetry.ts b/src/background/telemetry.ts index a7e1c8db18..d3eda3a2e2 100644 --- a/src/background/telemetry.ts +++ b/src/background/telemetry.ts @@ -29,7 +29,7 @@ import { count as registrySize } from "@/registry/packageRegistry"; import { count as logSize } from "@/telemetry/logging"; import { count as traceSize } from "@/telemetry/trace"; import { getUUID } from "@/telemetry/telemetryHelpers"; -import { getTabsWithAccess } from "@/utils/extensionUtils"; +import { getExtensionVersion, getTabsWithAccess } from "@/utils/extensionUtils"; import { type Event } from "@/telemetry/events"; const EVENT_BUFFER_DEBOUNCE_MS = 2000; @@ -274,7 +274,8 @@ export async function TEST_flushAll(): Promise { async function collectUserSummary(): Promise { const { os } = await browser.runtime.getPlatformInfo(); - const { version, version_name: versionName } = browser.runtime.getManifest(); + const { version_name: versionName } = browser.runtime.getManifest(); + const version = getExtensionVersion(); // Not supported on Chromium, and may require additional permissions // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/getBrowserInfo // const {name: browserName} = await browser.runtime.getBrowserInfo(); @@ -336,11 +337,9 @@ export async function recordEvent({ data: UnknownObject | undefined; }): Promise { if (await allowsTrack()) { - const { - version, - version_name: versionName, - manifest_version: manifestVersion, - } = browser.runtime.getManifest(); + const { version_name: versionName, manifest_version: manifestVersion } = + browser.runtime.getManifest(); + const version = getExtensionVersion(); const telemetryEvent = { uid: await getUUID(), event, diff --git a/src/contentScript/contentScriptPlatform.ts b/src/contentScript/contentScriptPlatform.ts index c15915ebbc..908f33fd42 100644 --- a/src/contentScript/contentScriptPlatform.ts +++ b/src/contentScript/contentScriptPlatform.ts @@ -49,7 +49,6 @@ import { writeToClipboard } from "@/utils/clipboardUtils"; import { snippetRegistry } from "@/contentScript/snippetShortcutMenu/snippetShortcutMenuController"; import BackgroundLogger from "@/telemetry/BackgroundLogger"; import * as sidebarController from "@/contentScript/sidebarController"; -import { validateSemVerString } from "@/types/helpers"; import type { UUID } from "@/types/stringTypes"; import { PlatformBase } from "@/platform/platformBase"; import type { Nullishable } from "@/utils/nullishUtils"; @@ -61,6 +60,7 @@ import { InteractiveLoginRequiredError } from "@/errors/authErrors"; import { deferLogin } from "@/contentScript/integrations/deferredLoginController"; import { flagOn } from "@/auth/featureFlagStorage"; import { selectionMenuActionRegistry } from "@/contentScript/textSelectionMenu/selectionMenuController"; +import { getExtensionVersion } from "@/utils/extensionUtils"; /** * @file Platform definition for mods running in a content script @@ -91,10 +91,7 @@ class ContentScriptPlatform extends PlatformBase { }); constructor() { - super( - "contentScript", - validateSemVerString(browser.runtime.getManifest().version), - ); + super("contentScript", getExtensionVersion()); } override capabilities: PlatformCapability[] = [ diff --git a/src/data/service/errorService.ts b/src/data/service/errorService.ts index 7c0e464bd8..5b43e09930 100644 --- a/src/data/service/errorService.ts +++ b/src/data/service/errorService.ts @@ -22,7 +22,7 @@ import { selectSpecificError, } from "@/errors/errorHelpers"; import { allowsTrack } from "@/telemetry/dnt"; -import { uuidv4, validateSemVerString } from "@/types/helpers"; +import { uuidv4 } from "@/types/helpers"; import { getUserData } from "@/auth/authStorage"; import { isAppRequestError, @@ -42,6 +42,7 @@ import { isObject } from "@/utils/objectUtils"; import type { Timestamp } from "@/types/stringTypes"; import { flagOn } from "@/auth/featureFlagStorage"; import { selectAbsoluteUrl } from "@/utils/urlUtils"; +import { getExtensionVersion } from "@/utils/extensionUtils"; const EVENT_BUFFER_DEBOUNCE_MS = 2000; const EVENT_BUFFER_MAX_MS = 10_000; @@ -75,9 +76,8 @@ async function flush(): Promise { export async function selectExtraContext( error: Error | SerializedError, ): Promise { - const { version, manifest_version: manifestVersion } = - browser.runtime.getManifest(); - const extensionVersion = validateSemVerString(version); + const { manifest_version: manifestVersion } = browser.runtime.getManifest(); + const extensionVersion = getExtensionVersion(); const extraContext: UnknownObject & { extensionVersion: SemVerString } = { extensionVersion, manifestVersion, diff --git a/src/extensionConsole/pages/UpdateBanner.tsx b/src/extensionConsole/pages/UpdateBanner.tsx index 23b9d01492..4c189c3af0 100644 --- a/src/extensionConsole/pages/UpdateBanner.tsx +++ b/src/extensionConsole/pages/UpdateBanner.tsx @@ -22,13 +22,14 @@ import reportError from "@/telemetry/reportError"; import Banner from "@/components/banner/Banner"; import { gt } from "semver"; import useAsyncState from "@/hooks/useAsyncState"; +import { getExtensionVersion } from "@/utils/extensionUtils"; // XXX: move this kind of async state to the Redux state. export function useUpdateAvailable(): boolean { const { data: updateAvailable } = useAsyncState(async () => { try { const available = await getAvailableVersion(); - const installed = browser.runtime.getManifest().version; + const installed = getExtensionVersion(); return available && installed !== available && gt(available, installed); } catch (error) { reportError(error); diff --git a/src/extensionConsole/pages/mods/modals/convertToRecipeModal/ConvertToRecipeModalBody.tsx b/src/extensionConsole/pages/mods/modals/convertToRecipeModal/ConvertToRecipeModalBody.tsx index dee4d8fffc..395e770853 100644 --- a/src/extensionConsole/pages/mods/modals/convertToRecipeModal/ConvertToRecipeModalBody.tsx +++ b/src/extensionConsole/pages/mods/modals/convertToRecipeModal/ConvertToRecipeModalBody.tsx @@ -26,7 +26,7 @@ import * as Yup from "yup"; import { PACKAGE_REGEX, testIsSemVerString, - validateSemVerString, + normalizeSemVerString, } from "@/types/helpers"; import { pick } from "lodash"; import Form from "@/components/form/Form"; @@ -136,7 +136,7 @@ const ConvertToRecipeModalBody: React.FunctionComponent = () => { () => ({ blueprintId: generatePackageId(scope, extension.label), name: extension.label, - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), description: "Created with the PixieBrix Page Editor", }), // eslint-disable-next-line react-hooks/exhaustive-deps -- initial values for the form, we calculate them once diff --git a/src/extensionConsole/pages/settings/useDiagnostics.ts b/src/extensionConsole/pages/settings/useDiagnostics.ts index 378e820d3b..2f9fee6529 100644 --- a/src/extensionConsole/pages/settings/useDiagnostics.ts +++ b/src/extensionConsole/pages/settings/useDiagnostics.ts @@ -21,7 +21,7 @@ import useExtensionPermissions, { type DetailedPermissions, } from "@/permissions/useExtensionPermissions"; import { type UnresolvedModComponent } from "@/types/modComponentTypes"; -import { compact, pick, uniqBy } from "lodash"; +import { compact, uniqBy } from "lodash"; import { type StorageEstimate } from "@/types/browserTypes"; import { count as registrySize } from "@/registry/packageRegistry"; import { count as logSize } from "@/telemetry/logging"; @@ -30,6 +30,7 @@ import { count as eventsSize } from "@/background/telemetry"; import useUserAction from "@/hooks/useUserAction"; import download from "downloadjs"; import filenamify from "filenamify"; +import { getExtensionVersion } from "@/utils/extensionUtils"; async function collectDiagnostics({ extensions, @@ -38,10 +39,11 @@ async function collectDiagnostics({ extensions: UnresolvedModComponent[]; permissions: DetailedPermissions; }) { - const manifest = browser.runtime.getManifest(); + const { version_name } = browser.runtime.getManifest(); + const version = getExtensionVersion(); return { userAgent: window.navigator.userAgent, - manifest: pick(manifest, ["version", "version_name"]), + manifest: { version, version_name }, permissions, storage: { storageEstimate: (await navigator.storage.estimate()) as StorageEstimate, diff --git a/src/extensionPages/extensionPagePlatform.ts b/src/extensionPages/extensionPagePlatform.ts index fc4b37844e..0ba10c8f8e 100644 --- a/src/extensionPages/extensionPagePlatform.ts +++ b/src/extensionPages/extensionPagePlatform.ts @@ -19,7 +19,6 @@ import { type PlatformProtocol } from "@/platform/platformProtocol"; import { hideNotification, showNotification } from "@/utils/notify"; import type { PlatformCapability } from "@/platform/capabilities"; import BackgroundLogger from "@/telemetry/BackgroundLogger"; -import { validateSemVerString } from "@/types/helpers"; import type { UUID } from "@/types/stringTypes"; import { traces, @@ -33,6 +32,7 @@ import type { NetworkRequestConfig } from "@/types/networkTypes"; import type { RemoteResponse } from "@/types/contract"; import integrationRegistry from "@/integrations/registry"; import { performConfiguredRequest } from "@/background/requests"; +import { getExtensionVersion } from "@/utils/extensionUtils"; /** * The extension page platform. @@ -56,10 +56,7 @@ class ExtensionPagePlatform extends PlatformBase { }); constructor() { - super( - "extension", - validateSemVerString(browser.runtime.getManifest().version), - ); + super("extension", getExtensionVersion()); } override alert = window.alert; diff --git a/src/pageEditor/panes/save/saveHelpers.test.ts b/src/pageEditor/panes/save/saveHelpers.test.ts index 9653d4d777..059ccec207 100644 --- a/src/pageEditor/panes/save/saveHelpers.test.ts +++ b/src/pageEditor/panes/save/saveHelpers.test.ts @@ -22,7 +22,7 @@ import { replaceModComponent, selectExtensionPointIntegrations, } from "@/pageEditor/panes/save/saveHelpers"; -import { validateRegistryId, validateSemVerString } from "@/types/helpers"; +import { validateRegistryId, normalizeSemVerString } from "@/types/helpers"; import menuItemExtensionAdapter from "@/pageEditor/starterBricks/menuItem"; import { internalStarterBrickMetaFactory, @@ -190,7 +190,7 @@ describe("replaceModComponent round trip", () => { metadata: { id: makeInternalId(modDefinition.definitions.extensionPoint), name: "Internal Starter Brick", - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), }, } as any); @@ -240,7 +240,7 @@ describe("replaceModComponent round trip", () => { metadata: { id: makeInternalId(modDefinition.definitions.extensionPoint), name: "Internal Starter Brick", - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), }, } as any); @@ -300,7 +300,7 @@ describe("replaceModComponent round trip", () => { metadata: { id: makeInternalId(modDefinition.definitions.extensionPoint), name: "Internal Starter Brick", - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), }, } as any); diff --git a/src/pageEditor/sidebar/ModListItem.test.tsx b/src/pageEditor/sidebar/ModListItem.test.tsx index e65369c906..c73ad116da 100644 --- a/src/pageEditor/sidebar/ModListItem.test.tsx +++ b/src/pageEditor/sidebar/ModListItem.test.tsx @@ -23,7 +23,7 @@ import { render } from "@/pageEditor/testHelpers"; import { Accordion, ListGroup } from "react-bootstrap"; import { appApiMock } from "@/testUtils/appApiMock"; import { modDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories"; -import { validateSemVerString } from "@/types/helpers"; +import { normalizeSemVerString } from "@/types/helpers"; describe("ModListItem", () => { it("renders expanded", async () => { @@ -99,7 +99,7 @@ describe("ModListItem", () => { const modDefinition = modDefinitionFactory({ metadata: { ...modMetadata, - version: validateSemVerString("1.0.1"), + version: normalizeSemVerString("1.0.1"), }, }); appApiMock diff --git a/src/pageEditor/sidebar/modals/CreateModModal.tsx b/src/pageEditor/sidebar/modals/CreateModModal.tsx index d0ca20756b..f2f6006e6d 100644 --- a/src/pageEditor/sidebar/modals/CreateModModal.tsx +++ b/src/pageEditor/sidebar/modals/CreateModModal.tsx @@ -20,7 +20,7 @@ import { PACKAGE_REGEX, testIsSemVerString, validateRegistryId, - validateSemVerString, + normalizeSemVerString, } from "@/types/helpers"; import { useDispatch, useSelector } from "react-redux"; import { @@ -88,7 +88,7 @@ function useInitialFormState({ return { id: newModId, name: `${modMetadata.name} (Copy)`, - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), description: modMetadata.description, }; } @@ -98,7 +98,7 @@ function useInitialFormState({ return { id: generatePackageId(scope, activeElement.label), name: activeElement.label, - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), description: "Created with the PixieBrix Page Editor", }; } diff --git a/src/pageEditor/starterBricks/base.ts b/src/pageEditor/starterBricks/base.ts index 11dc13c687..3a92333891 100644 --- a/src/pageEditor/starterBricks/base.ts +++ b/src/pageEditor/starterBricks/base.ts @@ -34,7 +34,7 @@ import { isInnerDefinitionRegistryId, uuidv4, validateRegistryId, - validateSemVerString, + normalizeSemVerString, } from "@/types/helpers"; import { type BrickPipeline, @@ -316,7 +316,7 @@ export function baseSelectExtensionPoint( id: metadata.id, // The server requires the version to save the brick, even though it's not marked as required // in the front-end schemas - version: metadata.version ?? validateSemVerString("1.0.0"), + version: metadata.version ?? normalizeSemVerString("1.0.0"), name: metadata.name, // The server requires the description to save the brick, even though it's not marked as required // in the front-end schemas diff --git a/src/platform/platformContext.ts b/src/platform/platformContext.ts index 1ab1695704..2a2c66bcae 100644 --- a/src/platform/platformContext.ts +++ b/src/platform/platformContext.ts @@ -21,14 +21,14 @@ import { PlatformCapabilityNotAvailableError, } from "@/platform/capabilities"; import { PlatformBase } from "@/platform/platformBase"; -import { validateSemVerString } from "@/types/helpers"; +import { normalizeSemVerString } from "@/types/helpers"; /** * A platform protocol with no available capabilities. */ export const uninitializedPlatform = new PlatformBase( "uninitialized", - validateSemVerString("0.0.0"), + normalizeSemVerString("0.0.0"), ); /** diff --git a/src/registry/packageRegistry.test.ts b/src/registry/packageRegistry.test.ts index cfd60d5bfb..4cfeee83d1 100644 --- a/src/registry/packageRegistry.test.ts +++ b/src/registry/packageRegistry.test.ts @@ -26,7 +26,7 @@ import { produce } from "immer"; import { appApiMock } from "@/testUtils/appApiMock"; import { defaultModDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories"; import pDefer from "p-defer"; -import { validateSemVerString } from "@/types/helpers"; +import { normalizeSemVerString } from "@/types/helpers"; describe("localRegistry", () => { beforeEach(() => { @@ -65,7 +65,7 @@ describe("localRegistry", () => { it("should return latest version", async () => { const definition = defaultModDefinitionFactory(); const updated = produce(definition, (draft) => { - draft.metadata.version = validateSemVerString("9.9.9"); + draft.metadata.version = normalizeSemVerString("9.9.9"); }); appApiMock.onGet("/api/registry/bricks/").reply(200, [updated, definition]); diff --git a/src/runtime/pipelineTests/component.test.ts b/src/runtime/pipelineTests/component.test.ts index 76b799d48f..21f919dd8d 100644 --- a/src/runtime/pipelineTests/component.test.ts +++ b/src/runtime/pipelineTests/component.test.ts @@ -26,7 +26,7 @@ import { } from "./pipelineTestHelpers"; import { fromJS } from "@/bricks/transformers/brickFactory"; -import { validateSemVerString } from "@/types/helpers"; +import { normalizeSemVerString } from "@/types/helpers"; import { TEST_setContext } from "webext-detect-page"; import { toExpression } from "@/utils/expressionUtils"; @@ -43,7 +43,7 @@ const componentBlock = fromJS(blockRegistry, { metadata: { id: "test/component", name: "Component Brick", - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), description: "Component block using v1 runtime", }, inputSchema: { diff --git a/src/testUtils/factories/brickFactories.ts b/src/testUtils/factories/brickFactories.ts index c5e3696fe1..7f1f9110f9 100644 --- a/src/testUtils/factories/brickFactories.ts +++ b/src/testUtils/factories/brickFactories.ts @@ -23,7 +23,7 @@ import { registryIdSequence, uuidSequence, } from "@/testUtils/factories/stringFactories"; -import { validateSemVerString } from "@/types/helpers"; +import { normalizeSemVerString } from "@/types/helpers"; import { emptyPermissionsFactory } from "@/permissions/permissionsUtils"; import { minimalSchemaFactory } from "@/utils/schemaUtils"; import type { BrickDefinition } from "@/bricks/transformers/brickFactory"; @@ -35,7 +35,7 @@ import type { ModDefinition } from "@/types/modDefinitionTypes"; export const brickFactory = define({ id: registryIdSequence, name: derive((x: Brick) => `Brick ${x.id}`, "id"), - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), inputSchema: minimalSchemaFactory, permissions: emptyPermissionsFactory, run: jest.fn(), diff --git a/src/testUtils/factories/deploymentFactories.ts b/src/testUtils/factories/deploymentFactories.ts index 0f57a116c5..2e95bc5d12 100644 --- a/src/testUtils/factories/deploymentFactories.ts +++ b/src/testUtils/factories/deploymentFactories.ts @@ -21,7 +21,7 @@ import { type ActivatableDeployment } from "@/types/deploymentTypes"; import { uuidSequence } from "@/testUtils/factories/stringFactories"; import { validateRegistryId, - validateSemVerString, + normalizeSemVerString, validateTimestamp, } from "@/types/helpers"; import { defaultModDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories"; @@ -32,7 +32,7 @@ import { type ModDefinition } from "@/types/modDefinitionTypes"; const deploymentPackageFactory = define({ id: uuidSequence, name: "Test Starter Brick", - version: (n: number) => validateSemVerString(`1.0.${n}`), + version: (n: number) => normalizeSemVerString(`1.0.${n}`), package_id: (n: number) => validateRegistryId(`test/starter-brick-${n}`), }); diff --git a/src/testUtils/factories/metadataFactory.ts b/src/testUtils/factories/metadataFactory.ts index b3a12c2792..54a603e57c 100644 --- a/src/testUtils/factories/metadataFactory.ts +++ b/src/testUtils/factories/metadataFactory.ts @@ -17,11 +17,11 @@ import { define } from "cooky-cutter"; import { type Metadata } from "@/types/registryTypes"; -import { validateRegistryId, validateSemVerString } from "@/types/helpers"; +import { validateRegistryId, normalizeSemVerString } from "@/types/helpers"; export const metadataFactory = define({ id: (n: number) => validateRegistryId(`test/mod-${n}`), name: (n: number) => `Mod ${n}`, description: "Mod generated from factory", - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), }); diff --git a/src/testUtils/platformMock.ts b/src/testUtils/platformMock.ts index 2a0f70b9b7..15a25d778b 100644 --- a/src/testUtils/platformMock.ts +++ b/src/testUtils/platformMock.ts @@ -21,7 +21,7 @@ import ConsoleLogger from "@/utils/ConsoleLogger"; import type { Logger } from "@/types/loggerTypes"; import { SimpleEventTarget } from "@/utils/SimpleEventTarget"; import type { RunArgs } from "@/types/runtimeTypes"; -import { validateSemVerString } from "@/types/helpers"; +import { normalizeSemVerString } from "@/types/helpers"; import type { ToastProtocol } from "@/platform/platformTypes/toastProtocol"; /** @@ -29,7 +29,7 @@ import type { ToastProtocol } from "@/platform/platformTypes/toastProtocol"; */ export const platformMock: PlatformProtocol = { platformName: "mock", - version: validateSemVerString("0.0.0"), + version: normalizeSemVerString("0.0.0"), capabilities: platformCapabilities, open: jest.fn(), alert: jest.fn(), diff --git a/src/testUtils/testAfterEnv.ts b/src/testUtils/testAfterEnv.ts index da5f8c4b6c..99483e84d4 100644 --- a/src/testUtils/testAfterEnv.ts +++ b/src/testUtils/testAfterEnv.ts @@ -35,7 +35,8 @@ browser.runtime.onMessage.addListener = jest.fn(); browser.runtime.id = "mpjjildTESTIDkfjnepo"; browser.runtime.getManifest = jest.fn().mockReturnValue({ - version: "1.5.2", + // Validate that 4-digit version numbers are supported + version: "1.5.2.3000", }); browser.runtime.getURL = (path) => `chrome-extension://abcxyz/${path}`; diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index d112349d5c..6ac3d9a6be 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -4,6 +4,7 @@ "strictNullChecks": true }, "files": [ + "./types/helpers.test.ts", "../end-to-end-tests/auth.setup.ts", "../end-to-end-tests/env.ts", "../end-to-end-tests/fixtures/extensionBase.ts", diff --git a/src/types/helpers.test.ts b/src/types/helpers.test.ts new file mode 100644 index 0000000000..a52101d95b --- /dev/null +++ b/src/types/helpers.test.ts @@ -0,0 +1,197 @@ +/* + * Copyright (C) 2024 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { testIsSemVerString, normalizeSemVerString } from "@/types/helpers"; + +describe("types/helpers.ts", () => { + describe("testIsSemVerString", () => { + it.each([ + { + value: "1.2.3", + allowLeadingV: false, + coerce: false, + expected: true, + }, + { + value: "1.2.3", + allowLeadingV: true, + coerce: true, + expected: true, + }, + { value: "v1.2.3", allowLeadingV: true, coerce: false, expected: true }, + { value: "v1.2.3", allowLeadingV: true, coerce: true, expected: true }, + { value: "v1.2.3", allowLeadingV: false, coerce: false, expected: false }, + { + value: "1.2.3.4000", + allowLeadingV: false, + coerce: false, + expected: false, + }, + { + value: "1.2.3.4000", + allowLeadingV: false, + coerce: true, + expected: true, + }, + { + value: "v1.2.3.4000", + allowLeadingV: true, + coerce: true, + expected: true, + }, + { + value: "lorem ipsum", + allowLeadingV: false, + coerce: false, + expected: false, + }, + { + value: "lorem ipsum", + allowLeadingV: false, + coerce: true, + expected: false, + }, + { + value: "", + allowLeadingV: false, + coerce: false, + expected: false, + }, + { + value: "", + allowLeadingV: false, + coerce: true, + expected: false, + }, + { + value: "vacant", + allowLeadingV: true, + coerce: false, + expected: false, + }, + { + value: "vacant", + allowLeadingV: true, + coerce: true, + expected: false, + }, + ])( + "$value with allowLeadingV: $allowLeadingV and coerce: $coerce returns $expected", + ({ value, allowLeadingV, coerce, expected }) => { + expect(testIsSemVerString(value, { allowLeadingV, coerce })).toBe( + expected, + ); + }, + ); + }); + + describe("normalizeSemVerString", () => { + it.each([ + { + value: "1.2.3", + allowLeadingV: false, + coerce: false, + expected: "1.2.3", + }, + { + value: "v1.2.3", + allowLeadingV: true, + coerce: false, + expected: "v1.2.3", + }, + { + value: "1.2.3.4000", + allowLeadingV: false, + coerce: true, + expected: "1.2.3", + }, + { + value: "v1.2.3.4000", + allowLeadingV: true, + coerce: true, + expected: "v1.2.3", + }, + ])( + "$value with allowLeadingV: $allowLeadingV and coerce: $coerce returns $expected", + ({ value, allowLeadingV, coerce, expected }) => { + expect(normalizeSemVerString(value, { allowLeadingV, coerce })).toBe( + expected, + ); + }, + ); + + it.each([ + { + value: "v1.2.3", + allowLeadingV: false, + coerce: false, + }, + { + value: "1.2.3.4000", + allowLeadingV: false, + coerce: false, + }, + { + value: "v1.2.3.4000", + allowLeadingV: false, + coerce: true, + }, + { + value: "v1.2.3.4000", + allowLeadingV: true, + coerce: false, + }, + { + value: "lorem ipsum", + allowLeadingV: false, + coerce: false, + }, + { + value: "lorem ipsum", + allowLeadingV: false, + coerce: true, + }, + { + value: "", + allowLeadingV: false, + coerce: false, + }, + { + value: "", + allowLeadingV: false, + coerce: true, + }, + { + value: "vacant", + allowLeadingV: true, + coerce: false, + }, + { + value: "vacant", + allowLeadingV: true, + coerce: true, + }, + ])( + "$value with allowLeadingV: $allowLeadingV and coerce: $coerce throws an error", + ({ value, allowLeadingV, coerce }) => { + expect(() => + normalizeSemVerString(value, { allowLeadingV, coerce }), + ).toThrow(); + }, + ); + }); +}); diff --git a/src/types/helpers.ts b/src/types/helpers.ts index a1776d8478..8f815b4945 100644 --- a/src/types/helpers.ts +++ b/src/types/helpers.ts @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -import { valid as semVerValid } from "semver"; +import { valid as semVerValid, coerce as semVerCoerce } from "semver"; import { startsWith } from "lodash"; import { @@ -151,10 +151,19 @@ export function validateTimestamp(value: string): Timestamp { throw new TypeError("Invalid timestamp"); } -export function validateSemVerString( +/** + * @param value The string to normalize + * @param allowLeadingV If `true`, a leading `v` is allowed. This results in a semver string that is not actually valid + * @param coerce If `true`, the string will be coerced to a valid semver string. See https://www.npmjs.com/package/semver#coercion + * @returns A normalized semver string + */ +export function normalizeSemVerString( value: string, // Default to `false` to be stricter. - { allowLeadingV = false }: { allowLeadingV?: boolean } = {}, + { + allowLeadingV = false, + coerce = false, + }: { allowLeadingV?: boolean; coerce?: boolean } = {}, ): SemVerString { if (value == null) { // We don't have strictNullChecks on, so null values will find there way here. We should pass them along. Eventually @@ -162,7 +171,16 @@ export function validateSemVerString( return value as SemVerString; } - if (testIsSemVerString(value, { allowLeadingV })) { + if (testIsSemVerString(value, { allowLeadingV, coerce })) { + if (coerce) { + const coerced = semVerValid(semVerCoerce(value)); + if (value.startsWith("v")) { + return `v${coerced}` as SemVerString; + } + + return coerced as SemVerString; + } + return value; } @@ -175,9 +193,14 @@ export function testIsSemVerString( value: string, // FIXME: the SemVerString type wasn't intended to support a leading `v`. See documentation // Default to `false` to be stricter. - { allowLeadingV = false }: { allowLeadingV?: boolean } = {}, + { + allowLeadingV = false, + coerce = false, + }: { allowLeadingV?: boolean; coerce?: boolean } = {}, ): value is SemVerString { - if (semVerValid(value) != null) { + const _value = coerce ? semVerCoerce(value) : value; + + if (semVerValid(_value) != null) { return allowLeadingV || !startsWith(value, "v"); } diff --git a/src/utils/deploymentUtils.test.ts b/src/utils/deploymentUtils.test.ts index 2c97242627..55ce6ba8d8 100644 --- a/src/utils/deploymentUtils.test.ts +++ b/src/utils/deploymentUtils.test.ts @@ -25,7 +25,7 @@ import { import { uuidv4, validateRegistryId, - validateSemVerString, + normalizeSemVerString, validateTimestamp, } from "@/types/helpers"; import { type SanitizedIntegrationConfig } from "@/integrations/integrationTypes"; @@ -42,6 +42,7 @@ import { PIXIEBRIX_INTEGRATION_ID, } from "@/integrations/constants"; import getModDefinitionIntegrationIds from "@/integrations/util/getModDefinitionIntegrationIds"; +import { getExtensionVersion } from "@/utils/extensionUtils"; describe("makeUpdatedFilter", () => { test.each([[{ restricted: true }, { restricted: false }]])( @@ -122,7 +123,7 @@ describe("makeUpdatedFilter", () => { _recipe: { ...modDefinition.metadata, // The factory produces version "1.0.1" - version: validateSemVerString("1.0.1"), + version: normalizeSemVerString("1.0.1"), updated_at: validateTimestamp(deployment.updated_at), // `sharing` doesn't impact the predicate. Pass an arbitrary value sharing: undefined, @@ -151,9 +152,8 @@ describe("checkExtensionUpdateRequired", () => { test("update not required", () => { const { deployment, modDefinition } = activatableDeploymentFactory(); - (modDefinition.metadata.extensionVersion as any) = `>=${ - browser.runtime.getManifest().version - }`; + (modDefinition.metadata.extensionVersion as any) = + `>=${getExtensionVersion()}`; expect( checkExtensionUpdateRequired([{ deployment, modDefinition }]), diff --git a/src/utils/deploymentUtils.ts b/src/utils/deploymentUtils.ts index 27300ce05e..cd32cc310b 100644 --- a/src/utils/deploymentUtils.ts +++ b/src/utils/deploymentUtils.ts @@ -30,6 +30,7 @@ import { type Except } from "type-fest"; import { PIXIEBRIX_INTEGRATION_ID } from "@/integrations/constants"; import getUnconfiguredComponentIntegrations from "@/integrations/util/getUnconfiguredComponentIntegrations"; import type { ActivatableDeployment } from "@/types/deploymentTypes"; +import { getExtensionVersion } from "@/utils/extensionUtils"; /** * Returns `true` if a managed deployment is active (i.e., has not been remotely paused by an admin) @@ -124,7 +125,8 @@ export function checkExtensionUpdateRequired( activatableDeployments: ActivatableDeployment[] = [], ): boolean { // Check that the user's extension can run the deployment - const { version: extensionVersion } = browser.runtime.getManifest(); + + const extensionVersion = getExtensionVersion(); const versionRanges = compact( activatableDeployments.map( ({ modDefinition }) => modDefinition.metadata.extensionVersion, diff --git a/src/utils/extensionUtils.ts b/src/utils/extensionUtils.ts index 41c0d317cb..69faeda8fc 100644 --- a/src/utils/extensionUtils.ts +++ b/src/utils/extensionUtils.ts @@ -20,6 +20,8 @@ import { foreverPendingPromise } from "@/utils/promiseUtils"; import { type Promisable } from "type-fest"; import { isScriptableUrl } from "webext-content-scripts"; import { type Runtime } from "webextension-polyfill"; +import { normalizeSemVerString } from "@/types/helpers"; +import { type SemVerString } from "@/types/registryTypes"; export const SHORTCUTS_URL = "chrome://extensions/shortcuts"; type Command = "toggle-quick-bar"; @@ -65,8 +67,23 @@ export function getExtensionConsoleUrl(page?: string): string { return url.href; } -export function getExtensionVersion(): string { - return browser.runtime.getManifest().version; +/** + * Gets the Extension version from the manifest and normalizes it to a valid semver string. + * @since 1.8.13, the Extension version is a four part format x.x.x.x + * This allows us to publish pre-release versions to the CWS, especially the BETA listing + * Each version published in CWS must have a unique version number + * + * @see manifest.mjs:getVersion() + * + * TODO: Add linting rule to prefer getExtensionVersion over browser.runtime.getManifest().version + * @see https://github.com/pixiebrix/pixiebrix-extension/issues/8349 + * + * @returns the version of the Extension in valid semver format (x.x.x) + */ +export function getExtensionVersion(): SemVerString { + return normalizeSemVerString(browser.runtime.getManifest().version, { + coerce: true, + }); } /** If no update is available and downloaded yet, it will return a string explaining why */ diff --git a/src/utils/objToYaml.test.ts b/src/utils/objToYaml.test.ts index 3ff5a8456d..08a420e768 100644 --- a/src/utils/objToYaml.test.ts +++ b/src/utils/objToYaml.test.ts @@ -16,7 +16,7 @@ */ import { brickToYaml } from "./objToYaml"; -import { validateSemVerString } from "@/types/helpers"; +import { normalizeSemVerString } from "@/types/helpers"; describe("brickToYaml", () => { test("serializes arbitrary object", () => { @@ -40,7 +40,7 @@ lorem: ipsum metadata: { id: "google/api", name: "Google API", - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), description: "Generic Google API authentication via API key", }, apiVersion: "v1", @@ -104,7 +104,7 @@ outputSchema: metadata: { id: "google/api", name: "Google API", - version: validateSemVerString("1.0.0"), + version: normalizeSemVerString("1.0.0"), description: "Generic Google API authentication via API key", }, apiVersion: "v1",