chore: revamp how we export types (#2118)

Author: SethFalco

.gitattributes

@@ -1,2 +1,6 @@
 # Enforce Unix newlines
 * text=auto eol=lf
+
+# https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
+/.yarn/releases/** binary
+/.yarn/plugins/** binary

.github/workflows/main.yml

@@ -24,7 +24,18 @@ jobs:
           cache: yarn
       - run: yarn install
       - run: yarn lint
-      - run: yarn typecheck
+      - run: yarn test:types
+  types:
+    name: Types
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: ${{ env.NODE }}
+          cache: yarn
+      - run: yarn install
+      - run: yarn test:types
   test:
     name: ${{ matrix.os }} Node.js ${{ matrix.node-version }}
     strategy:

.gitignore

@@ -1,13 +1,30 @@
-.pnp.*
+# https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
 .yarn/*
 !.yarn/patches
 !.yarn/plugins
 !.yarn/releases
 !.yarn/sdks
 !.yarn/versions
 
-node_modules
+# https://github.com/github/gitignore/blob/main/Global/Vim.gitignore
+[._]*.s[a-v][a-z]
+!*.svg
+[._]*.sw[a-p]
+[._]s[a-rt-v][a-z]
+[._]ss[a-gi-z]
+[._]sw[a-p]
+Session.vim
+Sessionx.vim
+.netrwhist
+*~
+tags
+[._]*.un~
+
+# Ignore generated files for browser, CommonJS, or types
 dist
+types
+
+node_modules
 test/regression-fixtures
 test/regression-diffs
 test/cli/output
@@ -16,11 +33,3 @@ coverage
 .vscode
 *.log
 package-lock.json
-
-# https://github.com/github/gitignore/blob/main/Global/Vim.gitignore
-[._]*.s[a-v][a-z]
-!*.svg  # comment out if you don't need vector files
-[._]*.sw[a-p]
-[._]s[a-rt-v][a-z]
-[._]ss[a-gi-z]
-[._]sw[a-p]

CONTRIBUTING.md

@@ -21,7 +21,7 @@ See: [SECURITY.md](./SECURITY.md)
 ### Requirements
 
 - [Git](https://git-scm.com/)
-- [Node.js 20](https://nodejs.org/) or later
+- [Node.js >=16](https://nodejs.org/) — Our CI pipeline includes Node.js v16, so even development code must be runnable in a Node.js v16 environment.
 
 ### Getting Started
 
@@ -55,7 +55,7 @@ You should read our [Plugin Architecture](https://svgo.dev/docs/plugins-api/) do
 
 SVGO plugins can optionally have parameters. These can be consumed by the plugin to tailor the behavior.
 
-Parameters must have types declared in a [`@typedef`](https://jsdoc.app/tags-typedef) at the top of the file. For new plugins, you must also append the appropriate type in [`plugins/plugin-types.d.ts`](https://github.com/svg/svgo/blob/main/plugins/plugins-types.d.ts). This way built-in plugins will have code completion and type checking as you'd expect while editing the plugin.
+Parameters must have types declared in a [`@typedef`](https://jsdoc.app/tags-typedef) at the top of the file. For new plugins, you must also append the appropriate type in [`lib/types.d.ts`](https://github.com/svg/svgo/blob/main/lib/types.d.ts). This way built-in plugins will have code completion and type checking as you'd expect while editing the plugin.
 
 ## Documentation
 

cspell.jsonc

@@ -47,7 +47,13 @@
     "Dmitry",
     "Keerthi",
   ],
-  "ignorePaths": ["*.svg.txt", "*.svg", "cspell.jsonc", "LICENSE"],
+  "ignorePaths": [
+    ".gitignore",
+    "*.svg.txt",
+    "*.svg",
+    "cspell.jsonc",
+    "LICENSE",
+  ],
   "ignoreRegExpList": [
     "-moz", // Mozilla Firefox CSS prefix
     "'.+?.svg',", // path to file with .svg extension

eslint.config.mjs

@@ -1,11 +1,7 @@
 import js from '@eslint/js';
 import globals from 'globals';
 
-/**
- * @typedef {import('eslint').Linter.Config} Config
- */
-
-/** @type {Config[]} */
+/** @type {import('eslint').Linter.Config[]} */
 export default [
   {
     ignores: [

lib/builtin.js

@@ -53,7 +53,13 @@ import * as reusePaths from '../plugins/reusePaths.js';
 import * as sortAttrs from '../plugins/sortAttrs.js';
 import * as sortDefsChildren from '../plugins/sortDefsChildren.js';
 
-export const builtin = Object.freeze([
+/**
+ * Plugins that are bundled with SVGO. This includes plugin presets, and plugins
+ * that are not enabled by default.
+ *
+ * @type {ReadonlyArray<{[Name in keyof import('./types.js').PluginsParams]: import('./types.js').BuiltinPluginOrPreset<Name, import('./types.js').PluginsParams[Name]>;}[keyof import('./types.js').PluginsParams]>}
+ */
+export const builtinPlugins = Object.freeze([
   presetDefault,
   addAttributesToSVGElement,
   addClassesToSVGElement,

lib/parser.js

@@ -1,16 +1,3 @@
-/**
- * @typedef {import('./types.js').XastNode} XastNode
- * @typedef {import('./types.js').XastInstruction} XastInstruction
- * @typedef {import('./types.js').XastDoctype} XastDoctype
- * @typedef {import('./types.js').XastComment} XastComment
- * @typedef {import('./types.js').XastRoot} XastRoot
- * @typedef {import('./types.js').XastElement} XastElement
- * @typedef {import('./types.js').XastCdata} XastCdata
- * @typedef {import('./types.js').XastText} XastText
- * @typedef {import('./types.js').XastParent} XastParent
- * @typedef {import('./types.js').XastChild} XastChild
- */
-
 import SAX from 'sax';
 import { textElems } from '../plugins/_collections.js';
 
@@ -20,9 +7,9 @@ export class SvgoParserError extends Error {
    * @param {number} line
    * @param {number} column
    * @param {string} source
-   * @param {string | undefined} file
+   * @param {string=} file
    */
-  constructor(message, line, column, source, file = undefined) {
+  constructor(message, line, column, source, file) {
     super(message);
     this.name = 'SvgoParserError';
     this.message = `${file || '<input>'}:${line}:${column}: ${message}`;
@@ -88,26 +75,26 @@ const config = {
  *
  * @param {string} data
  * @param {string=} from
- * @returns {XastRoot}
+ * @returns {import('./types.js').XastRoot}
  */
 export const parseSvg = (data, from) => {
   const sax = SAX.parser(config.strict, config);
-  /** @type {XastRoot} */
+  /** @type {import('./types.js').XastRoot} */
   const root = { type: 'root', children: [] };
-  /** @type {XastParent} */
+  /** @type {import('./types.js').XastParent} */
   let current = root;
-  /** @type {XastParent[]} */
+  /** @type {import('./types.js').XastParent[]} */
   const stack = [root];
 
   /**
-   * @param {XastChild} node
+   * @param {import('./types.js').XastChild} node
    */
   const pushToContent = (node) => {
     current.children.push(node);
   };
 
   sax.ondoctype = (doctype) => {
-    /** @type {XastDoctype} */
+    /** @type {import('./types.js').XastDoctype} */
     const node = {
       type: 'doctype',
       // TODO parse doctype for name, public and system to match xast
@@ -129,7 +116,7 @@ export const parseSvg = (data, from) => {
   };
 
   sax.onprocessinginstruction = (data) => {
-    /** @type {XastInstruction} */
+    /** @type {import('./types.js').XastInstruction} */
     const node = {
       type: 'instruction',
       name: data.name,
@@ -139,7 +126,7 @@ export const parseSvg = (data, from) => {
   };
 
   sax.oncomment = (comment) => {
-    /** @type {XastComment} */
+    /** @type {import('./types.js').XastComment} */
     const node = {
       type: 'comment',
       value: comment.trim(),
@@ -148,7 +135,7 @@ export const parseSvg = (data, from) => {
   };
 
   sax.oncdata = (cdata) => {
-    /** @type {XastCdata} */
+    /** @type {import('./types.js').XastCdata} */
     const node = {
       type: 'cdata',
       value: cdata,
@@ -157,7 +144,7 @@ export const parseSvg = (data, from) => {
   };
 
   sax.onopentag = (data) => {
-    /** @type {XastElement} */
+    /** @type {import('./types.js').XastElement} */
     const element = {
       type: 'element',
       name: data.name,
@@ -176,14 +163,14 @@ export const parseSvg = (data, from) => {
     if (current.type === 'element') {
       // prevent trimming of meaningful whitespace inside textual tags
       if (textElems.has(current.name)) {
-        /** @type {XastText} */
+        /** @type {import('./types.js').XastText} */
         const node = {
           type: 'text',
           value: text,
         };
         pushToContent(node);
       } else if (/\S/.test(text)) {
-        /** @type {XastText} */
+        /** @type {import('./types.js').XastText} */
         const node = {
           type: 'text',
           value: text.trim(),

lib/path.js

@@ -5,12 +5,10 @@
 import { removeLeadingZero, toFixed } from './svgo/tools.js';
 
 /**
- * @typedef {import('./types.js').PathDataItem} PathDataItem
- * @typedef {import('./types.js').PathDataCommand} PathDataCommand
  * @typedef {'none' | 'sign' | 'whole' | 'decimal_point' | 'decimal' | 'e' | 'exponent_sign' | 'exponent'} ReadNumberState
  *
  * @typedef StringifyPathDataOptions
- * @property {ReadonlyArray<PathDataItem>} pathData
+ * @property {ReadonlyArray<import('./types.js').PathDataItem>} pathData
  * @property {number=} precision
  * @property {boolean=} disableSpaceAfterFlags
  */
@@ -40,7 +38,7 @@ const argsCountPerCommand = {
 
 /**
  * @param {string} c
- * @returns {c is PathDataCommand}
+ * @returns {c is import('./types.js').PathDataCommand}
  */
 const isCommand = (c) => {
   return c in argsCountPerCommand;
@@ -138,12 +136,12 @@ const readNumber = (string, cursor) => {
 
 /**
  * @param {string} string
- * @returns {PathDataItem[]}
+ * @returns {import('./types.js').PathDataItem[]}
  */
 export const parsePathData = (string) => {
-  /** @type {PathDataItem[]} */
+  /** @type {import('./types.js').PathDataItem[]} */
   const pathData = [];
-  /** @type {?PathDataCommand} */
+  /** @type {?import('./types.js').PathDataCommand} */
   let command = null;
   let args = /** @type {number[]} */ ([]);
   let argsCount = 0;

lib/path.test.js

@@ -1,9 +1,5 @@
 import { parsePathData, stringifyPathData } from './path.js';
 
-/**
- * @typedef {import('../lib/types.js').PathDataItem} PathDataItem
- */
-
 describe('parse path data', () => {
   it('should allow spaces between commands', () => {
     expect(parsePathData('M0 10 L \n\r\t20 30')).toStrictEqual([
@@ -154,7 +150,7 @@ describe('stringify path data', () => {
     ).toBe('M.1 1e-7 2 2');
   });
   it('should configure precision', () => {
-    /** @type {ReadonlyArray<PathDataItem>} */
+    /** @type {ReadonlyArray<import('../lib/types.js').PathDataItem>} */
     const pathData = [
       { command: 'M', args: [0, -1.9876] },
       { command: 'L', args: [0.3, 3.14159265] },
@@ -175,7 +171,7 @@ describe('stringify path data', () => {
     ).toBe('M0-2 0 3 0-3 100 200');
   });
   it('allows to avoid spaces after arc flags', () => {
-    /** @type {ReadonlyArray<PathDataItem>} */
+    /** @type {ReadonlyArray<import('../lib/types.js').PathDataItem>} */
     const pathData = [
       { command: 'M', args: [0, 0] },
       { command: 'A', args: [50, 50, 10, 1, 0, 0.2, 20] },