issue_comments: 1515694393

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions issue performed_via_github_app
https://github.com/simonw/datasette/pull/2052#issuecomment-1515694393 https://api.github.com/repos/simonw/datasette/issues/2052 1515694393 IC_kwDOBm6k_c5aV6k5 9020979 2023-04-20T04:25:55Z 2023-04-20T04:25:55Z CONTRIBUTOR

Thanks for the thoughtful review and generous examples @asg017 ! I'll make the changes you suggested soon. Bonus thoughts inlined below.

comments

These were very much appreciated, it's important to a plugin system that details like this feel right! I'll address them in batch later in the week.

I know TypeScript can be a little controversial

FWIW I am in favor of doing Typescript - I just wanted to keep the initial set of files in this PR as simple as possible to review. Really appreciate you scaffolding this initial set of types + I think it would be a welcome addition to maintain a set of types.d.ts files.

I'm entertaining the idea of writing the actual source code in Typescript as long as the compiled output is readable b/c it can be tricky to keep the types and plain JS files in sync. Curious if you have encountered projects that are good at preventing drift.

Maybe they should have more "action-y" names

This is a great observation. I'm inclined towards something like make* or build* since to me add* make me think the thing the method is attached to is being mutated, but I agree that any of these may be clearer than the current get* setup. I'll go through and update these.

Maybe we can make it easier to do pure-js datasette plugins?

I really like this idea! It'll be easier to get contributors if they don't have to touch the python side at all.

And then do the PERMITTED_VIEWS filtering in JS rather than Python.

One cost of doing this is that pages that won't use the JS would still have to load the unused code (given that I'm not sending up anything complex like lazy loading). But hopefully the manager core size is close to negligible, and it won't be a big deal. 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
 1651082214