From 5dfac06ec1454ee2ee17e9894468d0035d20fa57 Mon Sep 17 00:00:00 2001 From: John Resig Date: Tue, 21 May 2024 15:10:33 -0400 Subject: [PATCH] Allow all special characters in IDs, except whitespace. (#2231) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: This came up with the Perseus team, where they had some Unicode characters in an ID and that was throwing errors. I did some more research and learned that only whitespace is considered to be invalid, so I updated our logic around this. Issue: WB-1704 ## Test plan: I ran the tests and they passed. Author: jeresig Reviewers: jandrade Required Reviewers: Approved By: jandrade Checks: ✅ codecov/project, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2231 --- .changeset/famous-birds-compare.md | 5 +++++ .../__snapshots__/unique-id-factory.test.ts.snap | 2 -- .../src/util/__tests__/unique-id-factory.test.ts | 4 ++-- .../wonder-blocks-core/src/util/unique-id-factory.ts | 11 +++++------ 4 files changed, 12 insertions(+), 10 deletions(-) create mode 100644 .changeset/famous-birds-compare.md diff --git a/.changeset/famous-birds-compare.md b/.changeset/famous-birds-compare.md new file mode 100644 index 000000000..85141f22f --- /dev/null +++ b/.changeset/famous-birds-compare.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-core": patch +--- + +Allow all special characters in IDs, except whitespace. diff --git a/packages/wonder-blocks-core/src/util/__tests__/__snapshots__/unique-id-factory.test.ts.snap b/packages/wonder-blocks-core/src/util/__tests__/__snapshots__/unique-id-factory.test.ts.snap index cec92e621..6df3fa817 100644 --- a/packages/wonder-blocks-core/src/util/__tests__/__snapshots__/unique-id-factory.test.ts.snap +++ b/packages/wonder-blocks-core/src/util/__tests__/__snapshots__/unique-id-factory.test.ts.snap @@ -1,7 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`UniqueIDFactory #constructor special characters in scope, throws 1`] = `"Invalid factory scope: invalid$%^&*("`; - exports[`UniqueIDFactory #constructor whitespace in scope, throws 1`] = `"Invalid factory scope: not valid scope"`; exports[`UniqueIDFactory #constructor whitespace only scope, throws 1`] = `"Invalid factory scope: "`; diff --git a/packages/wonder-blocks-core/src/util/__tests__/unique-id-factory.test.ts b/packages/wonder-blocks-core/src/util/__tests__/unique-id-factory.test.ts index b07906745..cce544c11 100644 --- a/packages/wonder-blocks-core/src/util/__tests__/unique-id-factory.test.ts +++ b/packages/wonder-blocks-core/src/util/__tests__/unique-id-factory.test.ts @@ -45,7 +45,7 @@ describe("UniqueIDFactory", () => { expect(underTest).toThrowErrorMatchingSnapshot(); }); - test("special characters in scope, throws", () => { + test("special characters in scope, should not throw", () => { // Arrange const scope = "invalid$%^&*("; @@ -53,7 +53,7 @@ describe("UniqueIDFactory", () => { const underTest = () => new UniqueIDFactory(scope); // Assert - expect(underTest).toThrowErrorMatchingSnapshot(); + expect(underTest).not.toThrow(); }); }); diff --git a/packages/wonder-blocks-core/src/util/unique-id-factory.ts b/packages/wonder-blocks-core/src/util/unique-id-factory.ts index bdd96d87f..253267842 100644 --- a/packages/wonder-blocks-core/src/util/unique-id-factory.ts +++ b/packages/wonder-blocks-core/src/util/unique-id-factory.ts @@ -33,14 +33,13 @@ export default class UniqueIDFactory implements IIdentifierFactory { * identifier. It does not assert that a string IS a valid identifier (for * example, that it doesn't start with numbers). We don't need to do that * here because all identifiers are prefixed to avoid needing that check. + * + * According to this post: + * https://stackoverflow.com/questions/70579/html-valid-id-attribute-values/31773673#31773673 + * The only characters that are not allowed in an id are whitespace characters. */ _hasValidIdChars(value?: string | null): boolean { - if (typeof value !== "string") { - return false; - } - - const invalidCharsReplaced = value.replace(/[^\d\w-]/g, "-"); - return value === invalidCharsReplaced; + return typeof value === "string" ? !/\s/.test(value) : false; } /**