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.

Sign up for GitHub

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

Jump to bottom

Publish types for setProperty #20

Open
sqs opened this issue May 12, 2019 · 4 comments
Open

Publish types for setProperty #20

sqs opened this issue May 12, 2019 · 4 comments
Assignees
@aeschli
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Milestone
Backlog

Comments

@sqs
Copy link
Contributor

sqs commented May 12, 2019

We use the setProperty function by itself. Right now, we use a fork of node-jsonc-parser because the main package doesn't publish types for non-main modules. Would you accept a PR that publishes types for all of the .js files in the npm package?

We use setProperty for the quick action buttons in the JSON settings editors on Sourcegraph, in case anyone's curious about the specific use case:
image
@sqs sqs mentioned this issue May 12, 2019
Merged
@aeschli
Copy link
Contributor

aeschli commented May 13, 2019

No problem for me to make the rewriter APIs public. It should be done by adding it to the main.ts class. The files in impl are considered implementation details so no d.ts for that.

@aeschli
Copy link
Contributor

aeschli commented May 13, 2019

I'm happy to take a PR if you want to give it a try.

@aeschli aeschli added feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities labels Oct 14, 2019
@aeschli aeschli added this to the Backlog milestone Oct 14, 2019
@yowayb
Copy link

yowayb commented Mar 5, 2022

setProperty() is exported as modify() with the exact same function signature, plus types. Please correct me if I'm wrong, but I think this means we should use modify() instead and discard this issue.

@aeschli
Copy link
Contributor

aeschli commented Mar 7, 2022

Yes, modify is the API that is exposed, and it what should be used. setProperty is an internal method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aeschli aeschli

Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@sqs @yowayb @aeschli