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

chore (fs-capacitor): types now shipped #62785

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Oct 18, 2022

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change.
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm test <package to test>.
  • If a package was never on Definitely Typed, you don't need to do anything. (If you wrote a package and provided types, you don't need to register it with us.)
  • Delete the package's directory.
  • Add it to notNeededPackages.json.

since 8.0.0 they ship their own types:
https://unpkg.com/browse/[email protected]/dist/index.d.ts

@typescript-bot
Copy link
Contributor

typescript-bot commented Oct 18, 2022

@43081j Thank you for submitting this PR!

This is a live comment which I will keep updated.

This PR touches some part of DefinitelyTyped infrastructure, so a DT maintainer will need to review it. This is rare — did you mean to do this?

2 packages in this PR (and infra files)

Code Reviews

Because this PR edits multiple packages, it can be merged once it's reviewed by a DT maintainer.

You can test the changes of this PR in the Playground.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ A DT maintainer needs to approve changes which affect DT infrastructure (notNeededPackages.json)

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 62785,
  "author": "43081j",
  "headCommitOid": "0611a5f101e2d431c8d6c42e7ebb50df2a99ac3b",
  "mergeBaseOid": "215916e23e32dd765aeb68a78d111a107b71ce88",
  "lastPushDate": "2022-10-18T21:38:37.000Z",
  "lastActivityDate": "2022-10-19T21:43:17.000Z",
  "mergeOfferDate": "2022-10-19T19:49:46.000Z",
  "mergeRequestDate": "2022-10-19T21:43:17.000Z",
  "mergeRequestUser": "43081j",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": null,
      "kind": "edit",
      "files": [
        {
          "path": "notNeededPackages.json",
          "kind": "infrastructure"
        }
      ],
      "owners": [],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "fs-capacitor",
      "kind": "delete",
      "files": [
        {
          "path": "types/fs-capacitor/fs-capacitor-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/fs-capacitor/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/fs-capacitor/tsconfig.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/fs-capacitor/tslint.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "mike-marcacci"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "graphql-upload",
      "kind": "edit",
      "files": [
        {
          "path": "types/graphql-upload/package.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "mike-marcacci"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "peterblazejewicz",
      "date": "2022-10-19T19:48:58.000Z",
      "isMaintainer": true
    }
  ],
  "mainBotCommentID": 1282993044,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Edits Infrastructure labels Oct 18, 2022
@typescript-bot
Copy link
Contributor

🔔 @mike-marcacci — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Oct 18, 2022
@typescript-bot
Copy link
Contributor

@43081j The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@43081j
Copy link
Contributor Author

43081j commented Oct 18, 2022

missed a dependent package, my mistake. may have to update the allowed dependencies list before this can be merged

microsoft/DefinitelyTyped-tools#548

@typescript-bot typescript-bot added Edits multiple packages and removed The CI failed When GH Actions fails labels Oct 18, 2022
@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Oct 18, 2022
@typescript-bot
Copy link
Contributor

@43081j The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@typescript-bot typescript-bot removed the The CI failed When GH Actions fails label Oct 18, 2022
Copy link
Member

@peterblazejewicz peterblazejewicz left a comment

Choose a reason for hiding this comment

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

LGTM!

@typescript-bot typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Oct 19, 2022
@43081j
Copy link
Contributor Author

43081j commented Oct 19, 2022

ready to merge 🥳

@typescript-bot typescript-bot merged commit 7cb75ce into DefinitelyTyped:master Oct 19, 2022
@43081j 43081j deleted the capacitor-nn branch October 19, 2022 21:45
Yogu added a commit to Yogu/graphql-upload that referenced this pull request Oct 20, 2022
The types have been removed from DefinitelyTyped in
DefinitelyTyped/DefinitelyTyped#62785

Version 8.0.0 is the stub package, and version 2.0.0 is the only other
version that exists, so pinning this version should not mess up any
dependency tree and just fix the type problem.
glasser pushed a commit to apollographql/graphql-upload that referenced this pull request Oct 20, 2022
The types have been removed from DefinitelyTyped in
DefinitelyTyped/DefinitelyTyped#62785

Version 8.0.0 is the stub package, and version 2.0.0 is the only other
version that exists, so pinning this version should not mess up any
dependency tree and just fix the type problem.
@L-U-C-K-Y
Copy link

We are using an older version of https://github.com/jaydenseric/graphql-upload as we cannot migrate to ESM just yet and by removing this type definitions, we are receiving this error:

error TS2688: Cannot find type definition file for 'fs-capacitor'.
   The file is in the program because:
     Entry point for implicit type library 'fs-capacitor'

@43081j
Copy link
Contributor Author

43081j commented Oct 23, 2022

did you update @types/fs-capacitor? the previous version will exist and work fine until you can upgrade fs-capacitor itself.

or maybe you don't use package-lock files?

basically the only way you should be able to get that error is by updating to the latest types package (which you shouldn't do unless you are ready to upgrade fs-capacitor too).

its also possible graphql-upload is the culprit and the version you're depending on forgot to include @types/fs-capacitor as a dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Edits Infrastructure Edits multiple packages Maintainer Approved Popular package This PR affects a popular package (as counted by NPM download counts). Self Merge This PR can now be self-merged by the PR author or an owner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants