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

update ts-standard to 12.0.0 #787

Closed
musoke opened this issue Apr 20, 2023 · 25 comments
Closed

update ts-standard to 12.0.0 #787

musoke opened this issue Apr 20, 2023 · 25 comments
Assignees
Milestone

Comments

@musoke
Copy link
Contributor

musoke commented Apr 20, 2023

Steps to Reproduce

  1. clone https://github.com/OpenBeta/open-tacos/
  2. yarn install
  3. yarn lint

Expected Behavior

package versions in package.json are mutually compatible.

Current Behavior

Get warning

=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <4.5.0

YOUR TYPESCRIPT VERSION: 4.9.5

Please only submit bug reports when using the officially supported version.

=============
Done in 9.33s.

Steps to fix

See also fix of similar issue in OpenBeta/openbeta-graphql#265

@musoke musoke added the good first issue Good for newcomers label Apr 20, 2023
@vnugent vnugent added this to the v0.9 milestone Apr 22, 2023
@actuallyyun
Copy link
Collaborator

Amazing! I have been seeing this error as well, thanks for fixing it in the other repo! @musoke I could open a PR to fix it in this repo if this sounds good to you guys. @vnugent

@actuallyyun
Copy link
Collaborator

Hey @musoke after running yarn fix, I still got many linting errors, and most of them are about this rule:https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-misused-promises.md

Screenshot 2023-04-27 at 16 51 59

Should I fix this error or ignore this rule?
There are also two errors about triple slashes :

  /Users/Ayun/webDev/OpenSource/open-tacos/next-env.d.ts:1:1: Do not use a triple slash reference for next, use `import` style instead. (@typescript-eslint/triple-slash-reference)
  /Users/Ayun/webDev/OpenSource/open-tacos/next-env.d.ts:2:1: Do not use a triple slash reference for next/image-types/global, use `import` style instead. (@typescript-eslint/triple-slash-reference)

But in the next-end.d.ts file, it writes

// NOTE: This file should not be edited
// see https://nextjs.org/docs/basic-features/typescript for more information.

@musoke
Copy link
Contributor Author

musoke commented Apr 27, 2023

This is probably a question for @vnugent, he wrote most of the code it still complains about

Triple slash imports: https://nextjs.org/docs/basic-features/typescript says that next-env.d.ts shouldn't be committed. @vnugent added it in f4c5c75 - is it ok to remove?

PS: probably best to time this carefully with the other incoming PRs, this touches enough files that it may conflict with a lot of them

@hawkishpolicy
Copy link
Contributor

Is this still an active issue?

@musoke
Copy link
Contributor Author

musoke commented May 12, 2023

@actuallyyun had a partial fix. I don't know if she is still working on it or it is up for grabs.

@vnugent
Copy link
Contributor

vnugent commented May 13, 2023

Triple slash imports: https://nextjs.org/docs/basic-features/typescript says that next-env.d.ts shouldn't be committed. @vnugent added it in f4c5c75 - is it ok to remove?

It was an oversight

is it ok to remove?

sure

@actuallyyun
Copy link
Collaborator

Sorry for the late reply. Hi @hawkishpolicy it is up for grabs! I started it but then I got many linting errors that require decisions beyond my ability. For instance, the no-misused-promises rule triggered a lot of errors all over the place. I am not sure if we should fix these errors or disable this rule. There are many other rules like this one.

@hawkishpolicy
Copy link
Contributor

hawkishpolicy commented May 16, 2023 via email

@vnugent
Copy link
Contributor

vnugent commented May 17, 2023

Added @hawkishpolicy to core-dev team commenting on the wrong issue

@hawkishpolicy
Copy link
Contributor

What's the best way to check if the script worked?

I followed the instructions provided and I'm not getting any errors but i'd like to double check before I submit a PR

@musoke
Copy link
Contributor Author

musoke commented May 25, 2023

I would delete node_modules, yarn install again and try yarn lint. One of us can also try it once you make a PR.

@musoke
Copy link
Contributor Author

musoke commented Jun 8, 2023

@hawkishpolicy, did that work for you?

@hawkishpolicy
Copy link
Contributor

hawkishpolicy commented Jun 8, 2023 via email

@musoke
Copy link
Contributor Author

musoke commented Jun 8, 2023

Ok, just wanted to make sure you're not stuck waiting for us.

@Ali7040
Copy link
Contributor

Ali7040 commented Jul 24, 2023

please assign it to me. I want to work on it

@vnugent vnugent modified the milestones: v0.9, v0.10 Jul 24, 2023
@hawkishpolicy
Copy link
Contributor

hawkishpolicy commented Jul 24, 2023 via email

@vnugent
Copy link
Contributor

vnugent commented Jul 26, 2023

@Ali7040 If you're working on this issue, please do a rebase with develop. I had to turn on Typescript strict mode for Issue #811 and ended up fixing / suppressing warnings in nearly 90 files.

@Ali7040
Copy link
Contributor

Ali7040 commented Jul 26, 2023

I am already working on develop branch.

@Ali7040
Copy link
Contributor

Ali7040 commented Jul 26, 2023

image
I just used yarn upgrade, and now everything works fine with "ts-standard": "11.0.0". Should I change it to "ts-standard": "12.0.0" or not?

@musoke
Copy link
Contributor Author

musoke commented Jul 26, 2023

@Ali7040, do you have a branch or draft PR you can share?

@Ali7040
Copy link
Contributor

Ali7040 commented Jul 26, 2023

@musoke I haven't created a PR yet.

@Ali7040
Copy link
Contributor

Ali7040 commented Jul 26, 2023

I just upgraded things, so it only changes the yarn.lock file. If you want, I can create a PR.

@musoke
Copy link
Contributor Author

musoke commented Jul 26, 2023

Please do

@Ali7040
Copy link
Contributor

Ali7040 commented Jul 26, 2023

Check it out: #939

@vnugent vnugent removed the good first issue Good for newcomers label Sep 21, 2023
@vnugent vnugent assigned vnugent and unassigned Ali7040 Sep 21, 2023
@vnugent
Copy link
Contributor

vnugent commented Sep 21, 2023

@Ali7040 thanks for giving this issue a try. It turns out more complicated than I thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

5 participants