Remove @vscode/sqlite3 from example and CI matrix

[belgattitude]

As per discovery on CI time, see #16638 (comment)

I'd like to propose to remove the @vscode/sqlite from the getting-started example. @vscode/sqlite looks to be a compatible fork of sqlite3 (node-sqlite) which is already supported by strapi and won't be touched in this pr. Diff
TryGhost/node-sqlite3@master...microsoft:vscode-node-sqlite3:release/vscode

The context

• the getting started install by default 3 drivers for sqlite (better-sqlite, sqlite3 and @vscode/sqlite).
• @vscode/sqlite is a slightly modified fork of sqlite3. They should be compatible.
• strapi prioritize the better-sqlite3 if present https://github.com/strapi/strapi/blob/main/packages/core/database/lib/connection.js#L38. Then @vscode/sqlite. Then sqlite3. Better-sqlite3 is probably the one most users have. Per install docs.
• in most situations (docker, ci, package manager...) the node-gyp based @vscode/sqlite will attempt to compile the extension which tend to take time and is a common cause of failures. For our ci and users

Reasons why ?

• On the ci yarn link step will run faster (the entire CI will probably save >45s per install, there's a lot of steps -> >45 seconds * steps)
• Remove one level of confusion for users.

The reason why not ?

• Maybe some users prefer to use this driver. But they have to be sure they don't have the better-sqlite3 installed... so anyway it looks counter-intuitive.

The possibles ?

• 1. Remove the driver from getting-started,
• 2. (exclude examples/* from the workspace and create a separate install that does not interfere with ou CI)

This PR suggest the first. It just removes the dep.

@gu-stav wdyt ?

[belgattitude]

See a run example https://github.com/strapi/strapi/actions/runs/4946607251/jobs/8845313813

---

---

[gu-stav]

@Marc-Roig @innerdvations I think it's a good idea to remove one of these drivers, if it helps to speed up the CI, but would want your opinion.

[innerdvations]

@Marc-Roig @innerdvations I think it's a good idea to remove one of these drivers, if it helps to speed up the CI, but would want your opinion.

I would absolutely love to, and it will certainly be removed in v5 (already discussed internally), but it's one of those things that would technically be a breaking change. Although I imagine the number of users affected would be incredibly small -- those running SQLite in production with the worst package option that hasn't been a Strapi default for over a year.

So we may want to go ahead and drop it anyway. Maybe 4.11 or 4.12 so it's at least a minor increase and not a patch.

[Marc-Roig]

@innerdvations doesnt this only affect the getstarted project and the CI? If I understand correctly strapi is only loading better-sqlite3 if it's installed so it does not make a difference no? https://github.com/strapi/strapi/blob/main/packages/core/database/lib/connection.js#L38

[belgattitude]

@Marc-Roig exactly.

[Marc-Roig]

haven't had the opportunity to say so, but THANK YOU for all the work you are doing to improve the CI @belgattitude 🚀

[innerdvations]

@innerdvations doesnt this only affect the getstarted project and the CI? If I understand correctly strapi is only loading better-sqlite3 if it's installed so it does not make a difference no? main/packages/core/database/lib/connection.js#L38

Short version: If we drop this package, we might as well drop the SQLite matrix to ['better-sqlite3'] at the same time because not having the package available will actually affect how the tests are run (it's a really bizarre case).

Long version: The whole topic of SQLite drivers is a huge mess that only made it into Strapi to prevent breaking changes with sqlite3 (which I wasn't around for, but have worked with several times), which is why I would be so glad to remove it.

Our CI is currently testing SQLite with the matrix ['better-sqlite3', 'sqlite3', '@vscode/sqlite3'] even though the last two are technically the same package. Because we have some logic to distinguish between them and actually load up the version built in to Knex when the package isn't available. By removing this package from getstarted, I'm pretty sure it means that our CI no longer has the package available, which actually affects which package is loaded by the CI when the tests are run. I think it means that those last two cases will start to be identical and therefore useless.

My personal opinion: We should go ahead and drop the SQLite matrix to only 'better-sqlite3` or maybe [better-sqlite3, sqlite3] (would have to investigate further to see which one makes sense) to save a bunch of CI time. Almost nobody is using the other edge cases anymore, and I also don't think I've seen a test fail on only those packages in the past ~9 months.

So, yes, I support this PR but I think we should go a step further because this has the unintended side effect of making one of our CI runs useless, somewhat removing support for an edge case.

[belgattitude]

Nice sum up. Thx.

I'll rescan the pr soon based on your comments. Maybe something pops up.

[belgattitude]

@innerdvations

• removed @vscode/sqlite3 from the matrix (I kept sqlite3 and better-sqlite3 unchanged) - 0861130

I agree with all you've said above :)

PS:

Idea for changelog (chatgpt)

---

• Deprecating @vscode/sqlite3 driver, recommending better-sqlite3, and providing migration instructions.

Description:

Deprecated the @vscode/sqlite3 driver and removed it from our QA tests. Please be aware that we'll officially remove the @vscode/sqlite3 driver support in an upcoming version.

However, support for SQLite is still and will remain available. We recommend using the better-sqlite3 driver, as stated in our documentation. If you are already using SQLite, it is likely that you are already using better-sqlite3.

To switch to better-sqlite3, please follow these steps:

Remove @vscode/sqlite3 using the command: yarn remove @vscode/sqlite3
Add better-sqlite3 using the command: yarn add better-sqlite3

Remember to always create a backup of your database before making any changes to prevent any potential issues or data loss.

---

Some notes

• @vscode/sqlite3 is a fork of node-sqlite3. They ensure compat, but looking at their github, I don't see users reporting on the @vscode/sqlite3 repo (33 github stars). Most "real-world" issues are reported in https://github.com/TryGhost/node-sqlite3/issues. My hypothesis is that very few people using the fork (outside of vscode tooling I guess). Diff TryGhost/node-sqlite3@master...microsoft:vscode-node-sqlite3:release/vscode - apart from improved error reporting, nothing seems breaking between the two

[belgattitude]

I would like to add a console.log('deprecated...') in https://github.com/strapi/strapi/blob/main/packages/core/database/lib/connection.js#L38. Is there a way to properly do it ? Would it make sense to create a link for support (ie: discussion or canonical issue)

It seems unlikely to me that this would break anything. Just the qa part removed. Plus we keep sqlite3 in the matrix which seems to be enough.

Once this pr is merged I'll create another one with the removal of sqlite3 in the qa. Then full removal (which might need a minor/major? version bump)

Wdyt ?

[innerdvations]

I'm ok with the scope of this PR, without adding a deprecation message. With how the SQLite packages are managed, I'm more worried that trying to add a deprecation message will cause it to appear when it shouldn't.

From what I can tell, @vscode/sqlite3 is only a wrapper around sqlite3 and in the end we're still testing sqlite3. And the vscode version is not necessary for the actual deployment of Strapi and I can't find anything documented anywhere (internally or publicly) that we ever supported it specifically, so I think it's fine that we don't test for it since it's using sqlite3 underneath and we're testing that. If someone wants to use it, they still can, it just comes with no support.

In v5 we intend to drop official support for the sqlite3 package as well, which will speed up the CI even more :)