latitude-dev / latitude

Developer-first embedded analytics
https://latitude.so
GNU Lesser General Public License v3.0
673 stars 26 forks source link

Fix: Latitude run watch not working #211

Closed csansoon closed 2 months ago

csansoon commented 2 months ago

Latitude run was working by invoking the query script in the server app. The invoking command was npm run query --watch <queryPath> <queryParams>. This invoked the script with the following arguments: ["--watch", <queryPath>, <queryParams>] (of course)

For some reason I do not understand, now npm decided not to pass dashed flags to the script, so it now was invoking the script with [<queryPath>, <queryParams>], completely ignoring the watch flag.

I have changed the script so that now it expects 3 arguments, instead of 2 arguments and a flag. Not it must be invoked like this: npm run query <queryPath> <watch:true|false> <queryParams>

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 92899b27e7091aeb66a6d9996696ea5df6c04157

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages | Name | Type | | --------------------- | ----- | | @latitude-data/cli | Patch | | @latitude-data/server | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

geclos commented 2 months ago

Try running the script like so:

npm run query -- --watch <queryPath> <queryParams>
csansoon commented 2 months ago

Try running the script like so:

npm run query -- --watch <queryPath> <queryParams>

@geclos Already tried, but didn't work. It was still removing the --watch flag

andresgutgon commented 2 months ago

The CLI has this at the beginning

#!/usr/bin/env node

Can you try to add it to the script. Maybe args are treated different

geclos commented 2 months ago

Try running the script like so:

npm run query -- --watch <queryPath> <queryParams>

@geclos Already tried, but didn't work. It was still removing the --watch flag

Hmm can we spend some time to try to understand why this is happening? Usually you should be able to pass flags to npm run commands by including the -- before arguments and I want to make sure it's not a bug we've introduced on our end.

csansoon commented 2 months ago

I see, I found the issue.

Apparently, running pnpm run query --watch queryPath queryParams does NOT skip the watch flag. But running the same command using npm instead, does.

What happened is that at first we were spawning the command using config.pkgManager.command, which for us that meant pnpm, but a few versions ago it was changed to be ran with npm. That's why it was working before but not now. Also, running it with -- does seem to work. Either way, I think it is best to just change the API for this script, as this is not meant to be ran directly by the user.

Also, I have modified the results_display.ts module, so that when imported it is not executed right away, as it was interfering with my debugging. Not it will only be instantiated when used instead.