Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strict typescript #79

Merged
merged 6 commits into from
May 12, 2022
Merged

Strict typescript #79

merged 6 commits into from
May 12, 2022

Conversation

berekuk
Copy link
Collaborator

@berekuk berekuk commented May 10, 2022

Finally :) 500+ errors cleaned up, "strict": true is now enabled in tsconfig.json.

Also:

  • save history simultaneously with question data (I wanted to do this anyway, and weird prisma types forced me to do this for this branch; one possible downside is that fetching a platform twice in a row now results in multiple history records in the same day, but that shouldn't usually happen, and if it happens because of manual runs of npm run cli PLATFORM, then it should also be easy to clean up manually with DELETE FROM history WHERE ...)
  • update squiggle (old squiggle-experimental didn't have type definitions)
  • minor refactorings; hope I didn't break any platform-fetching code, I had to refactor some of it blindly, but if something breaks, it'll be easy to fix
  • moved code for calculating stars into platform modules, as previously discussed (I forgot the comment where I mentioned it)

@vercel
Copy link

vercel bot commented May 10, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
metaforecast ✅ Ready (Inspect) Visit Preview May 12, 2022 at 2:00PM (UTC)

@NunoSempere
Copy link
Collaborator

Is this ready to be merged?

@berekuk
Copy link
Collaborator Author

berekuk commented May 11, 2022

Is this ready to be merged?

Almost, I wanted to figure out #80 first.

@berekuk
Copy link
Collaborator Author

berekuk commented May 12, 2022

One more significant change which I implemented as a side effect of smarkets investigation: platform fetchers now can return partial results, while the CLI tool can accept platform-specific arguments.

Example: npm run cli -- smarkets --eventId 34757345 --verbose 1

Platforms specify the list of arguments they accept, and there's some checking going on, both in runtime and with typescript (typescript side is a bit messy, but better than nothing).

Technical note: I had to expand on Platform type for this: there's now a version field on every platform object, and smarkets code uses v2 while everything else is still on v1. v2 platforms return { questions: [...], partial: true/false } objects, where partial flag being set means that old questions shouldn't be removed from the database.

@berekuk berekuk merged commit 6fbeea2 into master May 12, 2022
@berekuk
Copy link
Collaborator Author

berekuk commented May 12, 2022

Rolling back temporarily due to 500 error on frontpage. Investigating.

@berekuk
Copy link
Collaborator Author

berekuk commented May 12, 2022

It was a dumb circular dependency bug, fixed in master and re-deployed.

And then there was also an issue with replaceAll, which was missing in Node 14, and I had to switch Vercel to Node 16; @NunoSempere, please let me know if you think replaceAll polyfill is still necessary (I removed it), I think it's supported well enough by all modern browsers (maybe we should still support IE 11 or Edge <85 for some reason?..)

@NunoSempere
Copy link
Collaborator

NunoSempere commented May 12, 2022

let me know if you think replaceAll polyfill is still necessary

Doesn't seem necessary; legacy from the netlify days.

@berekuk berekuk deleted the stricter-typescript branch May 12, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants