eslint-plugin-drizzle add "db" as default
Is there a reason why it is automatically applied for every delete function? I think most users would use "db" const anyway.
If you decide not to make "db" default, can we somehow override this for all rules? it sounds very cumbersome to specify this for every rule (imagine if even more get added in the future)
19 Replies
cc: @Angelelz
We shipped it with not default and were hoping to get some feedback.
But there is an option
drizzleObjectName
where you can define itMy feedback is that I added it into my app, and it immediately broke my api routes that are named "delete", which should be pretty common in CRUD apps
but this needs to be declared for every rule separately, right?
TBH I think most people will only need it in the delete
I'm also not sure if using
db
as default might have unintended consecuences, when users don't notice that, use the plugin trusting it will yell at them on delete, and then finding out that it didn't work becuase their db object is named database
the current default also seems to have unintended consequences unfortunately
That's the kind of stuff we were afraid of when we publish. I think it's better to have a false positive and an false negative
That's my opinion but the feedback is welcome
I wish you could check somehow if the .delete method is called somehow on a drizzle db object or something similar, instead of manually specifying the variable name
I looked into that, I'm not sure yet if it's possible. If it is, it will involve including an additional parser and complications during testing.
I looked into other plugins and I couldn't find one that have a similar approach in the assertion
I think the biggest problem with the lint plugin is that your project is destined to break just by adding it. The only way to not have your app break, is by configuring the "drizzleObjectName" variable. But it is not plug-n-play without it. Normally, lint plugins work without any specific configuration.
and let's not even talk about changing the variable in the code, but forgetting about changing it in the eslintrc 😄 it is rare, but could also happen
but overall, great initiative!
Yep I get what you're saying. This is the first version though 👍
looking forward to how it plays out in the future then 😄
I'd suggest you use the beta version we just published this morning
right now it has a false negative if you use the
drizzleObjectName
like db, but it's a nested object like this.data.db.delete(...)
thanks!
@itsyoboieltr, thanks for your feedback!
We will improve ESLint to make it work as you mentioned and check the exact drizzle type. Still, I'm not sure how to make it work properly. Personally, I also needed this check on a project I'm working on, and releasing it in this state was a good solution for now. Yes, you need to specify a name for each rule, but we have only 2 of them. Before adding any new ones with the same problem, we will try to improve it. For now, I see the current setup as a good combination of the needed helper and time to develop it.
Also, if you have any expertise on how to check a specific type in TS AST, I would love to get any hints on how to do it
@Andrew Sherman Are you using it in the drizzle studio codebase? 😅
no 😅
You should, don't delete our precious data!
the current solution, is just putting it as part of the template I am developing, so everytime I start a new project, the rules are already there
https://github.com/itsyoboieltr/dbest-stack/blob/b44dbad9595cc2aeff27f807f441497eb78a3302/package.json#L70C2-L70C2
GitHub
dbest-stack/package.json at b44dbad9595cc2aeff27f807f441497eb78a330...
Contribute to itsyoboieltr/dbest-stack development by creating an account on GitHub.
Good stuff!