Skip to content
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

fix: readonly arrays on input options #1498

Merged
merged 4 commits into from
Feb 23, 2025

Conversation

Zamiell
Copy link
Contributor

@Zamiell Zamiell commented Feb 23, 2025

Today I discovered that npm-check-updates would not take in my immutable filterVersion array. So, I fixed it in this PR, as well as the surrounding arrays that were also incorrectly marked as mutable.

More info: In TypeScript libraries it is idiomatic to take in read-only data structures. That way, end-users do not have to cast their immutable data structures to be mutable simply to satisfy the TypeScript compiler (which is both ugly and reduces safety in using the library).

In case it isn't obvious, this PR is backwards compatible in that end-users can now use either mutable OR immutable arrays.

Copy link
Owner

@raineorshine raineorshine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, seems fine!

Despite its location in /src, the file RunOptions.ts is actually the output of an automated process that generates the types and documentation for the API. See this note at the top of the file (which is not entirely obvious):

/** This file is generated automatically from the options specified in /src/cli-options.ts. Do not edit manually. Run "npm run build" or "npm run build:options" to build. */

So instead of modifying RunOptions.ts directly, you will need to change https://github.com/raineorshine/npm-check-updates/blob/main/src/cli-options.ts and possibly https://github.com/raineorshine/npm-check-updates/blob/main/src/scripts/build-options.ts and then run build:options and commit the updated RunOptions.ts to achieve the desired change.

@Zamiell Zamiell requested a review from raineorshine February 23, 2025 17:39
@Zamiell
Copy link
Contributor Author

Zamiell commented Feb 23, 2025

Makes sense, please review the updated PR.

@raineorshine raineorshine merged commit de0c152 into raineorshine:main Feb 23, 2025
7 checks passed
@Zamiell Zamiell deleted the fix-typescript branch February 23, 2025 17:58
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.

2 participants