#140 [technical debt payment] use ink-picture, address vulnerabilities, and fix linter errors

[Anonymous]

Originally created by: endernoke

Roadmap: [#85]

Bug fixes / refactors

• Removed the in-house image components and use ink-picture instead, and removed a bunch of related dependencies
• Put feed MediaView in altscreen
• Fixed a unicode rendering bug. This could be reproduced by viewing posts with wide unicode characters like ⏳ in the caption)
• The bug was fixed in ink 6 but unfortunately we are stuck on Ink 5 because we depend on pastel
• A post-install hook is added which uses patch-package to patch the bug in Ink directly, using the diff inside patches/. This file is intentionally committed into git history so everyone can get the patch when they run npm install.

Pre-commit workflow, tests, and CI

• Made the prettier formatter and xo work with each other, previously they will try to overwrite each other's changes 💀
• Made the ava test functional. Currently it doesn't do much and is just a sanity check which verifies that the build is able to output stuff to stdout (see discussion below)
• Enabled linter in CI, and changed the node.js version in CI to a compatible version

Ultimate technical debt repayment

• Fixed all the xo linter errors
• Addressed some npm vulnerabilities, see [#129]. I think it's a bit challenging to bring number of vulnerabilities to 0 unless we contribute to upstream packages

---

The number of vulnerabilities has gone down from 25 to 6. Most of these are from instagram-typescript-api, which I just realized hasn't been updated for a year 💀 We could only let them sit there and rot. The other solution is to fork the repo and maintain it but we probably shouldn't do it unless a maintainer has a lot of free time lol

Fun fact: when you run npx xo on the current main branch (48db0431b183ba2f0cd15cf72f208072409dcfc9), you will get exactly 1181 errors.
With the changes now there's 0 errors. The people shall rejoice and celebrate this marvelous achievement!! 🥳

[Anonymous]

Originally posted by: supreme-gg-gg

@endernoke you are the GOAT and have saved us from the financial crisis. Can you enabled linter in precommit + CI since our code is passing now? It should just be uncommenting two blocks I left there. I'll have a look when you're ready

p.s. 10k LOC total in one PR is crazy bro we've broken another record

[Anonymous]

Originally posted by: endernoke

@endernoke you are the GOAT and have saved us from the financial crisis. Can you enabled linter in precommit + CI since our code is passing now? It should just be uncommenting two blocks I left there. I'll have a look when you're ready

p.s. 10k LOC total in one PR is crazy bro we've broken another record

sure will do tmr

[Anonymous]

Originally posted by: endernoke

@supreme-gg-gg could you merge this after testing and verify that all the ci and linting works thanks

[Anonymous]

Originally posted by: supreme-gg-gg

@endernoke you might need to rebase ts-migration/main on main and force push here again, it seems like there are a few commits from main included here

[Anonymous]

Originally posted by: endernoke

@endernoke you might need to rebase ts-migration/main on main and force push here again, it seems like there are a few commits from main included here

Lmao bro this is getting fun, somehow on my local commit history this branch is branched from latest ts-migration/main but it shows otherwise on github 💀 have you seen this before should I just rebase anyway

[Anonymous]

Originally posted by: supreme-gg-gg

@endernoke I updated the test.tsx to stop using App.tsx which is just sitting rotting there doing nothing so it's removed (tech debt --;)

Regarding writing real tests in the future, I think our structure now is a bit difficult to test the commands directly since the mock can only be used to test the Views, not the command like <Chat> directly because it doesn't take in a client as props. I think testing the views would be fine because what matters is that the view renders and matches what we expect for the types (whcih are in the mock). What do you think about this?

We don't really need complex UI test cases, we just need to ensure people don't end up breaking prod 😭

[Anonymous]

Originally posted by: supreme-gg-gg

Looks good I will merge it, should we add simple tests next what do you think?

Also I will create an issue to support Ink 6 on Pastel repo but it will 99.9% get ghosted, I suggest we just fork it and upgrade it to Ink 6 because Ink is very actively maintained with weekly releases, for long-term maintainability we shouldn't get stuck with Pastel which has not been maintained for half a year...

[Anonymous]

Ticket changed by: supreme-gg-gg

• status: open --> closed

[Anonymous]

Originally posted by: endernoke

@endernoke I updated the test.tsx to stop using App.tsx which is just sitting rotting there doing nothing so it's removed (tech debt --;)

Nice, I added your changes while rebasing and tagged u as co author

Regarding writing real tests in the future, I think our structure now is a bit difficult to test the commands directly since the mock can only be used to test the Views, not the command like <Chat> directly because it doesn't take in a client as props. I think testing the views would be fine because what matters is that the view renders and matches what we expect for the types (whcih are in the mock). What do you think about this?

We don't really need complex UI test cases, we just need to ensure people don't end up breaking prod 😭

Agreed, testing views with mock should be sufficient

Though I think the most important part of testing should be verifying interactivity, like making sure the thing doesn't break when you scroll up/down lmao, which is hard to test... Do you know of a way to test this?

[Anonymous]

Originally posted by: endernoke

Also I will create an issue to support Ink 6 on Pastel repo but it will 99.9% get ghosted, I suggest we just fork it and upgrade it to Ink 6 because Ink is very actively maintained with weekly releases, for long-term maintainability we shouldn't get stuck with Pastel which has not been maintained for half a year...

Sure go ahead let's farm more aura 🫡
I think pastel is now maintained by sindre sorhus, who is active, i just merged something with him a few days ago lol, so pastel might still have hope
Perhaps if we don't get a reply within a week then let's fork it

I'll also open an issue on instagram private api to update their dependencies and fork it if i get ghosted

[Anonymous]

Originally posted by: supreme-gg-gg

@endernoke I updated the test.tsx to stop using App.tsx which is just sitting rotting there doing nothing so it's removed (tech debt --;)

Nice, I added your changes while rebasing and tagged u as co author

Regarding writing real tests in the future, I think our structure now is a bit difficult to test the commands directly since the mock can only be used to test the Views, not the command like <Chat> directly because it doesn't take in a client as props. I think testing the views would be fine because what matters is that the view renders and matches what we expect for the types (whcih are in the mock). What do you think about this?
We don't really need complex UI test cases, we just need to ensure people don't end up breaking prod 😭

Agreed, testing views with mock should be sufficient

Though I think the most important part of testing should be verifying interactivity, like making sure the thing doesn't break when you scroll up/down lmao, which is hard to test... Do you know of a way to test this?

Yea that's what I meant we couldn't really do "UI testing", I'm sure we can always just input a set of commands, but idk how to check the results, perhaps reference how frontend does UI testing I have no knowledge of it though

[Anonymous]

Originally posted by: supreme-gg-gg

Also I will create an issue to support Ink 6 on Pastel repo but it will 99.9% get ghosted, I suggest we just fork it and upgrade it to Ink 6 because Ink is very actively maintained with weekly releases, for long-term maintainability we shouldn't get stuck with Pastel which has not been maintained for half a year...

Sure go ahead let's farm more aura 🫡 I think pastel is now maintained by sindre sorhus, who is active, i just merged something with him a few days ago lol, so pastel might still have hope Perhaps if we don't get a reply within a week then let's fork it

I'll also open an issue on instagram private api to update their dependencies and fork it if i get ghosted

oh that's nice to hear, let's hope it goes well!

[Anonymous]

Originally posted by: endernoke

@endernoke I updated the test.tsx to stop using App.tsx which is just sitting rotting there doing nothing so it's removed (tech debt --;)

Nice, I added your changes while rebasing and tagged u as co author

Regarding writing real tests in the future, I think our structure now is a bit difficult to test the commands directly since the mock can only be used to test the Views, not the command like <Chat> directly because it doesn't take in a client as props. I think testing the views would be fine because what matters is that the view renders and matches what we expect for the types (whcih are in the mock). What do you think about this?
We don't really need complex UI test cases, we just need to ensure people don't end up breaking prod 😭

Agreed, testing views with mock should be sufficient
Though I think the most important part of testing should be verifying interactivity, like making sure the thing doesn't break when you scroll up/down lmao, which is hard to test... Do you know of a way to test this?

Yea that's what I meant we couldn't really do "UI testing", I'm sure we can always just input a set of commands, but idk how to check the results, perhaps reference how frontend does UI testing I have no knowledge of it though

No experience with frontend testing too 😭 I think we can reference the tests in pastel I remember they have a way of inputting commands and checking results

Tbh do we really need tests for the ink frontend? Especially when the UI is going to be changed quite often lol. If it doesn't do much then let's not do it cuz it adds unnecessary complexity

[Anonymous]

Originally posted by: supreme-gg-gg

lol we absolutely do not need UI tests just using the mocks to test the views is enough, if we make breaking schema changes just update the mocks lol like minimum effort but avoid dumb mistakes