Skip to content

Conversation

@mureinik
Copy link
Contributor

Make sure all the functions documented in sqlite's documentation have CJS/ESM variants:

This PR includes:

  • Adding a CJS variant for createTagStore
  • Separating applyChangeset's code snippet to CJS and ESM variants and adding the missing import/require of DatabaseSync to them so it's executable without having to manually modify it.

Fixes: #60394

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. sqlite Issues and PRs related to the SQLite subsystem. labels Oct 24, 2025
@louwers
Copy link
Contributor

louwers commented Oct 24, 2025

Are documentation examples required to be self-contained? I had a discussion with @Renegade334 in #60200 (comment)

I think it's a good practice. It's a shame the whole snippet has to be repeated though.

@mureinik
Copy link
Contributor Author

mureinik commented Oct 24, 2025

@louwers:

Are documentation examples required to be self-contained? I had a discussion with @Renegade334 in #60200 (comment)

I admit I don't contribute enough to the Node.js project to have a strong opinion here (or for my opinion to carry much weight, TBH).
However, empirically, there are six code snippets that call new DatabaseSync in the sqlite documentation for Node.js 25.0.0.
Five of them contain an explicit require/import for it beforehand, and applyChangeset doesn't, so I figured that was an oversight and it would be better to follow suite.

I think it's a good practice.

Agreed

It's a shame the whole snippet has to be repeated though.

Agreed on this one too.
I'm not aware of a way to dedup the examples and share code between the CJS and ESM snippets, but maybe I'm missing something here.

Separate `applyChangeset`'s documentation to CJS and MJS examples. This
also allows adding the correct `import`/`require`, and make the
snippet executable without any additional "prep work".

Fixes: nodejs#60394
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Not all sqlite code snippet have CJS/ESM variants

5 participants