fast icon indicating copy to clipboard operation
fast copied to clipboard

Turn on `noImplicitAny` and `strictPropertyInitialization` for `fast-router`

Open chrisdholt opened this issue 4 years ago • 15 comments

As part of #5303 we turned off @typescript-eslint/typedef globally as it was overwritten across multiple packages. As part of that, we turned on noImplicitAny and strictPropertyInitialization for all packages. Turning this on revealed several issues related to both tsconfig settings in fast-router and so it was turned off explicitly for that package. The errors should be fixed and addressed as part of this issue and the code should be tested to ensure that there are no breaks or side effects of the change. This issue exists to track turning that back on and resolving the errors.

chrisdholt avatar Oct 28 '21 22:10 chrisdholt

Hi @chrisdholt, So as far as I can understand, while turning on noImplicitAny and strictPropertyInitialization, the issues related to both tsconfig settings in fast-router also need to be resolved. Right?

ritikBhandari avatar Mar 12 '22 08:03 ritikBhandari

Hi @chrisdholt, So as far as I can understand, while turning on noImplicitAny and strictPropertyInitialization, the issues related to both tsconfig settings in fast-router also need to be resolved. Right?

Yes, the goal for this would be to turn those settings on and resolve the issues related to them for the fast-router package.

chrisdholt avatar Mar 12 '22 08:03 chrisdholt

Would u mind if I try to resolve this issue?

ritikBhandari avatar Mar 12 '22 09:03 ritikBhandari

Turning this on revealed several issues related to both tsconfig settings in fast-router

@chrisdholt how should I reproduce these errors again? Did u find it in testing phase or any other case?

ritikBhandari avatar Mar 19 '22 14:03 ritikBhandari

Turning this on revealed several issues related to both tsconfig settings in fast-router

@chrisdholt how should I reproduce these errors again? Did u find it in testing phase or any other case?

If I recall, the issue should repro by turning on the settings in tsconfig and running the build and tests steps.

chrisdholt avatar Mar 19 '22 17:03 chrisdholt

During testing, all test cases are running except for fast-router. Is that the issue? Because right now lerna run test is working fine except for that particular folder.

ritikBhandari avatar Mar 19 '22 20:03 ritikBhandari

During testing, all test cases are running except for fast-router. Is that the issue?

No the issue is you would see errors thrown relating to instances of no-implicit-any and several cases where strictPropertyInitialization isn’t being done as expected. Not at my computer but I can try to give an example tomorrow (maybe this evening but will be afk for a bit).

chrisdholt avatar Mar 19 '22 20:03 chrisdholt

Sure. In the meanwhile i'll check why the tests are running with exception.

ritikBhandari avatar Mar 19 '22 20:03 ritikBhandari

@chrisdholt can you attach any example of the errors as I still cannot find them after turning on those parameters.

ritikBhandari avatar Mar 24 '22 10:03 ritikBhandari

fasterror

@chrisdholt Are we noticing these type of errors?

ritikBhandari avatar Mar 26 '22 18:03 ritikBhandari

fasterror

@chrisdholt Are we noticing these type of errors?

That looks like what I would reproduce - those appear to be noImplicitAny errors.

chrisdholt avatar Mar 31 '22 19:03 chrisdholt

So now we're sure these are the errors to be fixed right?

ritikBhandari avatar Mar 31 '22 19:03 ritikBhandari

So now we're sure these are the errors to be fixed right?

Yes, we want to fix the noImplicitAny typings, which is likely going to require adding the correct typings. That could open up potential type issues with the implementation where something may be inferred but could be undefined or may not overlap.

Probably best to get @EisenbergEffect to confirm, but I think a PR here for stricter typing would be an improvement, it'll likely just take some time to review depending on what all gets flagged as you move through fixing these.

chrisdholt avatar Mar 31 '22 19:03 chrisdholt

The router is still in a bit of a volatile stage so I haven't added all the typings. It can be easier to evolve something at first without that, adding it later once the code has matured. That said, I'd definitely take a PR to improve this now.

EisenbergEffect avatar Apr 01 '22 20:04 EisenbergEffect

Hey, is there anyone working on this issue? I'd like to give a try. @EisenbergEffect @chrisdholt

NiteshSethi09 avatar Aug 20 '22 20:08 NiteshSethi09