Update to sqlite3 5.1.6

[benallfree]

This PR brings @types/sqlite3 up to date with the latest version of the sqlite3 package.

sqlite3 ships with its own type definitions https://github.com/TryGhost/node-sqlite3/blob/master/lib/sqlite3.d.ts, so this is just a copy of that.

This @types/sqlite3 package remains important because the sqlite3 interface has become a defacto-standard and is used by projedts like node-sqlite. @types/sqlite3 makes it possible to reference the sqlite3 interface typings without pulling in the entirety of the sqlite3 package as a dependency. See kriasoft/node-sqlite#177 and kriasoft/node-sqlite#175 for further discussion.

I would also be interested in becoming an owner of @types/sqlite3 so I can keep it up to date with the sqlite3 project and even extend it with missing typings such as TryGhost/node-sqlite3#1726.

[typescript-bot]

@benallfree Thank you for submitting this PR!

This is a live comment which I will keep updated.

2 packages in this PR

• spatialite — on npm, on unpkg
• sqlite3 — on npm, on unpkg
  • 1 added owner: ✎@benallfree

Code Reviews

This PR can be merged once it's reviewed.

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

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• 🕐 Type definition owners or DT maintainers needs to approve changes which affect more than one package

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Inactive

This PR has been inactive for 31 days — it is considered abandoned, and therefore closed!

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 67290,
  "author": "benallfree",
  "headCommitOid": "1f38bbc61a41d87e2cb383df26782989f35781df",
  "mergeBaseOid": "a83d860873a3ca5d8bde53432560c23b5b9da180",
  "lastPushDate": "2023-11-02T20:09:52.000Z",
  "lastActivityDate": "2024-01-08T23:15:16.000Z",
  "maintainerBlessed": "Waiting for Code Reviews",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "spatialite",
      "kind": "edit",
      "files": [
        {
          "path": "types/spatialite/spatialite-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "atd-schubert"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "sqlite3",
      "kind": "edit",
      "files": [
        {
          "path": "types/sqlite3/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/sqlite3/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/sqlite3/sqlite3-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "nmalaguti",
        "dpyro"
      ],
      "addedOwners": [
        "benallfree"
      ],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [
    {
      "type": "changereq",
      "reviewer": "sheetalkamat",
      "date": "2024-01-08T23:15:16.000Z"
    },
    {
      "type": "approved",
      "reviewer": "theogravity",
      "date": "2023-11-23T19:12:49.000Z",
      "isMaintainer": false
    },
    {
      "type": "stale",
      "reviewer": "gabritto",
      "date": "2023-11-23T02:02:55.000Z",
      "abbrOid": "a98993b"
    }
  ],
  "mainBotCommentID": 1791471843,
  "ciResult": "pass"
}

[typescript-bot]

🔔 @atd-schubert @nmalaguti @dpyro — 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]

@benallfree 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]

@benallfree 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]

@benallfree 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]

@benallfree 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.

[benallfree]

@gabritto Added owner info and removed .d.ts header - thank you!

[typescript-bot]

@gabritto, @theogravity Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

[typescript-bot]

@gabritto Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

[jakebailey]

This @types/sqlite3 package remains important because the sqlite3 interface has become a defacto-standard and is used by projedts like node-sqlite. @types/sqlite3 makes it possible to reference the sqlite3 interface typings without pulling in the entirety of the sqlite3 package as a dependency.

Truthfully, I'm not sure that this is a good idea; if someone installs the real sqlite3 package, then that package's types will take precedence and the DT types won't work. So if the two are ever out of date, it will be misleading to anyone who has managed to only install the package from DT. Not having the DT package at all is a lot easier to reason about.

I'm also not sure what the point is of trying to make things that optimal; surely someone is going to have to install sqlite3 at some point, because otherwise why use the types at all? DT describes actual runtime values and shouldn't be used as grab bag of random types.

[theogravity]

I'm the maintainer of the sqlite package and I do not include but copy the sqlite3 types because I don't want to make sqlite3 a direct dependency of my project (so people can use the lib with a compatible API interface) but still need the types from it.

Having the types come from here allows me to use it as a dep without requiring the actual sqlite3 binaries and allows other developers who wish to extend the types to add their own functionality as the current way I do it (copy the type def file into my project) prevents that from working.

Not everyone installs sqlite3 with sqlite, but I don't know the stats around it. For example, there used to be a lib called sqlite3-offline that had a ton of pre-compiled sqlite versions for systems that can't build sqlite3. The way the sqlite is designed offers compatibility with any lib that conforms to the sqlite3 API.

I recommend reading the PR description and issues linked to get more context on why the change is being requested if you haven't yet.

[andrewbranch]

I think it’s a pretty bad idea to duplicate the types between the runtime package and DT. Instead, you might consider publishing a separate sqlite3-types or something.

[benallfree]

@andrewbranch Do you think this package should be deprecated then?

[andrewbranch]

Yes.

[andrewbranch]

Things will not get any worse by merging this PR, so I will approve it if everyone is intent on going that route. But I read all the linked discussions and am still convinced that this approach will be more harmful than helpful. It seems like publishing a separate package outside of DT would address @theogravity’s goals without duplicating code or being tangled up in TypeScript’s special module resolution behavior for @types packages, so I think that should be considered.

[benallfree]

@andrewbranch That's an interesting perspective, thank you. I've started a discussion on TryGhost/node-sqlite3#1732 to see if they have any interest in reviving @types/sqlite3 as the official source for typings.

[jakebailey]

I'm confused by the above; my reading is that Andrew was saying to not do that, and instead create an npm package not on DT that would contain the types, not keep this package as the official source.

[benallfree]

Only because sqlite3 already has the types in their repo. I was suggesting that if sqlite3 wanted to move their types back to here, then this could become the official package again rather than having a separate package.

At this point, it doesn't seem like anyone wants to move away from their approaches so perhaps deprecating this package is the best way to close this issue. (Although it won't solve the original problem)

[benine203]

there has got to be plans for making a build mode called 'upstream datatypes puller'
it gets the tagged version, append the .9999 and everything

[benallfree]

@benine203 Are you proposing that @types/sqlite3 be automatically updated whenever the sqlite3 types change?

[benine203]

yeah indeed so but doing so may need some non-existent build infrastructure capabilities. to kick a new upstream-based release in motion: a committer just initially converts a typing module from regular to upstream-based by supplying a set of patches or middleware to extract/mangle the upstream typedefs which don't change often but what changes on each PR is either a URL/sha256 pair like in BSD's pkgsrc or a remote/tag pair. the CI has added machinery to interpret this declarative update and add actual commits to the PR and after that it is like before and maintainers can fix/tweak before final acceptance. or it can be all updated on some periodic puller on a long-lived instance somewhere that ETLs the upstream source and files PRs. if issues in PRs arise sends out notifications to a list of module owners to come review. testing should be less involved as the upstream is supposed to have done this already.

…

On Sat, Dec 16, 2023, 19:49 Ben Allfree ***@***.***> wrote: @benine203 <https://github.com/benine203> Are you proposing that @types/sqlite3 be automatically updated whenever the sqlite3 types change? — Reply to this email directly, view it on GitHub <#67290 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BDODQ2HHA5XU4UBHZGDVCNLYJZTVNAVCNFSM6AAAAAA63OI3J2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJZGAZDSMRZGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[sheetalkamat]

It is bad idea to have duplicate packages and the preference is to maintain types outside DT. So this package needs to be removed from DT instead of accepting this PR.

[typescript-bot]

@benallfree One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

[typescript-bot]

@benallfree I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Feb 7th (in a week) if the issues aren't addressed.

[typescript-bot]

@benallfree To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!