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

PackageJson is not a valid JsonObject #79

Closed
ifiokjr opened this issue Feb 10, 2020 · 5 comments · Fixed by #465
Closed

PackageJson is not a valid JsonObject #79

ifiokjr opened this issue Feb 10, 2020 · 5 comments · Fixed by #465
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@ifiokjr
Copy link
Contributor

ifiokjr commented Feb 10, 2020

Description

The following fails type checking.

import {PackageJson, JsonObject} from 'type-fest';
import {expectType} from 'tsd';

const packageJson: PackageJson = {};

const createPackageJson = () => packageJson;
expectType<JsonObject>(createPackageJson()); // => Error

The error produced is the following.

Argument of type 'PackageJson' is not assignable to parameter of type 'JsonObject'.
  Index signatures are incompatible.
    Type 'unknown' is not assignable to type 'JsonValue'.
      Type 'unknown' is not assignable to type 'JsonArray'.ts(2345)

To fix this we would need to remove unknown from the PackageJson type and replace it with JsonValue since the package.json file only supports valid Json this shouldn't cause any issues.

I'll create a PR shortly.

@resynth1943
Copy link
Contributor

You closed your PRs. Want me to do it?

@ifiokjr
Copy link
Contributor Author

ifiokjr commented Feb 11, 2020

@resynth1943 if you have time. The PR's were failing since there's no JSON overlap with undefined.

I wasn't sure how to work around that limitation.

@resynth1943
Copy link
Contributor

I'll take a look tonight. ♥️

@voxpelli voxpelli added bug Something isn't working help wanted Extra attention is needed labels Sep 22, 2021
@voxpelli
Copy link
Collaborator

I think the approach you had @ifiokjr was a promising one.

Its either that or to extend the JsonValue to accept more things, but the latter doesn't sound right.

Would be happy to try and help get a PR reviewed and passing for this.

@skarab42
Copy link
Collaborator

I think the approach you had @ifiokjr was a promising one.

Yep it was really close ;) Thanks for the poc @ifiokjr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
4 participants