fix(cleanupIds): skip generated ids for malformed references (#1817)

I also had some chores pending which were trivial individually, I opted
to incorporate them here as well.

* In JSDocs, use the nullable syntax as it's more concise and familiar
from TypeScript usage.
* discord → Discord (proper nouns should match capitalization)
* Use actions/checkout v4 instead of v2

.github/ISSUE_TEMPLATE/config.yml

@@ -2,4 +2,4 @@ blank_issues_enabled: false
 contact_links:
   - name: Questions
     url: https://discord.gg/z8jX8NYxrE
-    about: Please ask questions in our discord server.
+    about: Please ask questions in our Discord server.

.github/workflows/main.yml

@@ -18,7 +18,7 @@ jobs:
     name: Lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - uses: actions/setup-node@v2
         with:
           node-version: ${{ env.NODE }}
@@ -30,7 +30,7 @@ jobs:
     name: Test regressions
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - uses: actions/setup-node@v2
         with:
           node-version: ${{ env.NODE }}
@@ -48,7 +48,7 @@ jobs:
           - 16
           - 14
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - uses: actions/setup-node@v2
         with:
           node-version: ${{ matrix.node-version }}

lib/path.js

@@ -68,7 +68,7 @@ const isDigit = (c) => {
  */
 
 /**
- * @type {(string: string, cursor: number) => [number, number | null]}
+ * @type {(string: string, cursor: number) => [number, ?number]}
  */
 const readNumber = (string, cursor) => {
   let i = cursor;
@@ -143,7 +143,7 @@ const parsePathData = (string) => {
    */
   const pathData = [];
   /**
-   * @type {null | PathDataCommand}
+   * @type {?PathDataCommand}
    */
   let command = null;
   let args = /** @type {number[]} */ ([]);

lib/stringifier.js

@@ -17,7 +17,7 @@ const { textElems } = require('../plugins/_collections.js');
 /**
  * @typedef {{
  *   indent: string,
- *   textContext: null | XastElement,
+ *   textContext: ?XastElement,
  *   indentLevel: number,
  * }} State
  */

lib/style.test.js

@@ -14,7 +14,7 @@ const { parseSvg } = require('./parser.js');
  */
 const getElementById = (node, id) => {
   /**
-   * @type {null | XastElement}
+   * @type {?XastElement}
    */
   let matched = null;
   visit(node, {

lib/xast.js

@@ -24,7 +24,7 @@ const querySelectorAll = (node, selector) => {
 exports.querySelectorAll = querySelectorAll;
 
 /**
- * @type {(node: XastNode, selector: string) => null | XastChild}
+ * @type {(node: XastNode, selector: string) => ?XastChild}
  */
 const querySelector = (node, selector) => {
   return selectOne(selector, node, cssSelectOptions);

lib/xast.test.js

@@ -17,7 +17,7 @@ const root = (children) => {
 /**
  * @type {(
  *   name: string,
- *   attrs?: null | Record<string, string>,
+ *   attrs?: ?Record<string, string>,
  *   children?: Array<XastElement>
  * ) => XastElement}
  */

plugins/_transforms.js

@@ -22,7 +22,7 @@ exports.transform2js = (transformString) => {
   const transforms = [];
   // current transform context
   /**
-   * @type {null | TransformItem}
+   * @type {?TransformItem}
    */
   let current = null;
   // split value into ['', 'translate', '10 50', '', 'scale', '2', '', 'rotate', '-45', '']

plugins/cleanupIds.js

@@ -87,7 +87,8 @@ const hasStringPrefix = (string, prefixes) => {
 /**
  * Generate unique minimal ID.
  *
- * @type {(currentId: null | Array<number>) => Array<number>}
+ * @param {?number[]} currentId
+ * @returns {number[]}
  */
 const generateId = (currentId) => {
   if (currentId == null) {
@@ -232,27 +233,28 @@ exports.fn = (_root, params) => {
           return;
         }
         /**
-         * @type {(id: string) => boolean}
-         **/
+         * @param {string} id
+         * @returns {boolean}
+         */
         const isIdPreserved = (id) =>
           preserveIds.has(id) || hasStringPrefix(id, preserveIdPrefixes);
-        /**
-         * @type {null | Array<number>}
-         */
+        /** @type {?number[]} */
         let currentId = null;
         for (const [id, refs] of referencesById) {
           const node = nodeById.get(id);
           if (node != null) {
             // replace referenced IDs with the minified ones
             if (minify && isIdPreserved(id) === false) {
-              /**
-               * @type {null | string}
-               */
+              /** @type {?string} */
               let currentIdString = null;
               do {
                 currentId = generateId(currentId);
                 currentIdString = getIdString(currentId);
-              } while (isIdPreserved(currentIdString));
+              } while (
+                isIdPreserved(currentIdString) ||
+                (referencesById.has(currentIdString) &&
+                  nodeById.get(currentIdString) == null)
+              );
               node.attributes.id = currentIdString;
               for (const { element, name } of refs) {
                 const value = element.attributes[name];

plugins/inlineStyles.js

@@ -109,7 +109,7 @@ exports.fn = (root, params) => {
           }
         }
         /**
-         * @type {null | csstree.CssNode}
+         * @type {?csstree.CssNode}
          */
         let cssAst = null;
         try {

plugins/mergeStyles.js

@@ -19,7 +19,7 @@ exports.description = 'merge multiple style elements into one';
  */
 exports.fn = () => {
   /**
-   * @type {null | XastElement}
+   * @type {?XastElement}
    */
   let firstStyleElement = null;
   let collectedStyles = '';

plugins/prefixIds.js

@@ -66,7 +66,7 @@ const prefixId = (prefixGenerator, body) => {
  *
  * @param {(id: string) => string} prefixGenerator Function to generate a prefix.
  * @param {string} reference An arbitrary string, should start with "#".
- * @returns {string|null} The given string with a prefix inserted, or null if the string did not start with "#".
+ * @returns {?string} The given string with a prefix inserted, or null if the string did not start with "#".
  */
 const prefixReference = (prefixGenerator, reference) => {
   if (reference.startsWith('#')) {
@@ -158,7 +158,7 @@ exports.fn = (_root, params, info) => {
             cssText = node.children[0].value;
           }
           /**
-           * @type {null | csstree.CssNode}
+           * @type {?csstree.CssNode}
            */
           let cssAst = null;
           try {

plugins/removeOffCanvasPaths.js

@@ -21,7 +21,7 @@ exports.description =
  */
 exports.fn = () => {
   /**
-   * @type {null | {
+   * @type {?{
    *   top: number,
    *   right: number,
    *   bottom: number,