Skip to content

chores: various clean ups regardin ESM and TypeScript #2117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 3, 2025

Conversation

SethFalco
Copy link
Member

@SethFalco SethFalco commented May 3, 2025

I've been spending a lot of time going through our types, reading through documentation, or some subjective best practices other developers have put together regarding how to define types.

I apply some of these here, plus a few other small chores.

Remove Main from package.json

Packages that don't support Node.js versions below v12 don't need to define both. The export property supersedes the main property, so the old property can be dropped entirely.

We would only need to preserve main if we're expected users to pull it in to Node.js v12, but our minimum supported version is Node.js v16.

The "exports" provides a modern alternative to "main" allowing multiple entry points to be defined, conditional entry resolution support between environments, and preventing any other entry points besides those defined in "exports".

https://docs.npmjs.com/cli/v11/configuring-npm/package-json (emphasis mine)

The primary difference between the two is that when using main, it exports all files in the package for public use. While exports enables us to define our public API, so that developers are less likely to depend on our internal API in their projects.

By defining a clear boundary between public and private API, users are less likely to encounter situations where a non-breaking change is breaking for them, because they depended on internal code.

Upgrade Types

Ideally, the @types/ package versions would match the major and minor version of the implementation library. This upgrades them to either match or at least be as close as possible.

DefinitelyTyped make the major and minor version match the implementation version, then the patch version is less relevant. This ensures we're linting against the correct types.

Each Definitely Typed package is versioned when published to npm. The DefinitelyTyped-tools (the tool that publishes @types packages to npm) will set the declaration package's version by using the major.minor.9999 version number listed in package.json.

https://github.com/DefinitelyTyped/DefinitelyTyped?tab=readme-ov-file#how-do-definitely-typed-package-versions-relate-to-versions-of-the-corresponding-library

Move comment to @fileoverview

I prefer when important information are maintained in JSDocs rather than comments. This one seemed fine to move to a JSDoc.

Prefer ReadonlyArray<> over [] for @params

This one I'm taking from the guidance for DefinitelyTyped. While we don't have to follow the same rules, I think it'd be nice to adhere to similar rules, and I see no harm in applying this change.

This can actually be better as before, while users could pass regular arrays to these functions, they could not pass readonly arrays. Now, a user can pass both to functions where we weren't modifying the array anyway.

Common mistakes

function sum(nums: number[]): number: Use ReadonlyArray if a function does not write to its parameters.

https://github.com/DefinitelyTyped/DefinitelyTyped?tab=readme-ov-file#common-mistakes

@SethFalco SethFalco merged commit 49954bc into svg:main May 3, 2025
13 checks passed
@SethFalco SethFalco deleted the esm-types branch May 3, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant