refactor: Fix TypeScript error that namespace Parse cannot be found

[dplewis]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

Ideally we would like to be backwards compatible with @types/parse. @types/parse uses namespaces while this SDK uses modules. This PR fixes Cannot find namespace "Parse" error.

Approach

• Add Parse namespace to types only
• Remove ParseType interface
• Add types path for parse/node and parse/react-native with tests
• Add missing types
• Update test:types script to include test error count

Types test errors left: 495 (from 585)

TODO: (Added to #2012 (comment))

• Backwards compatible with @types/parse/index.d.ts
  • Add generics to Parse.Object
  • Add generics to Parse.Query
  • Add generics to Parse.Installation
  • Add generics to Parse.Relation
  • Add generics to Parse.Schema
  • Add generics to Parse.Push
  • Add generics to Parse.Role
  • Add Parse.Cloud node / cloud code types
  • Add missing types where needed
  • Pass all tests

Tasks

• Add tests
• Add changes to documentation (guides, repository pages, code comments)

[parse-github-assistant]

Thanks for opening this pull request!

• ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (1fc520c) to head (8ef4c1c).
Report is 2 commits behind head on alpha.

Additional details and impacted files

@@            Coverage Diff            @@
##             alpha     #2452   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        64           
  Lines         6256      6256           
  Branches      1446      1476   +30     
=========================================
  Hits          6256      6256           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mtrezza]

What are we using test:types for? What's does that ensure?

[dplewis]

it ensures that our autogenerated types matches @types/parse

[dplewis]

@mtrezza Is this good to merge? I updated the CI, now we can see how many tests failed. When it reaches 0 is when we can publish types.

[mtrezza]

it ensures that our autogenerated types matches @types/parse

Could you help me understand why that is necessary? Aren't the generated types always correct, because they reflect the method and variable types as they are defined in code?

[dplewis]

If a developer has an existing typescript project that uses @types/parse then switch to our types this will cause a breaking change as is.

There are 495 errors that lets us know the generated types aren't correct.

[mtrezza]

But shouldn't our generated types always be correct? In your screenshot, if the type is any, then it's any, if it's void, then it's void. We define that type in code after all. Or what does the error in your screenshot tell us - that we have set the wrong type in code?

[dplewis]

I'm not a Typescript expert but I believe after this PR is merged you can publish the types if you want and incrementally add, which was always the original intention. The types test suite tells that the types in our code is wrong or missing types. We are doing this for the sake of completeness. I can foresee tons of users posting issues like #1368

[mtrezza]

Originally we didn't publish the types, because during the time of gradual conversion to TS the DT types were more complete had a greater benefit for developers. Had we published the types, they'd have been in conflict with DT and a developer would have had to remove the more complete TD types for our less complete own types.

We also said that once we achieve a decent type coverage, specifically replace enough of these any placeholders with correct types, we'll publish our types.

Now I understand that the types check here errors because we still have many of these any types. But simply searching the code base for any types will also tell us that we still have too many, even though there may be some places where they may be actually the correct types.

1. I doubt we'll ever align 100% with DT. I don't think that's necessary for publishing our own types, as long as our types provide a decent value for developers compared to DT. DT is currently missing some types and contains outdated types due to recent code changes on our side. So even though we may use many any types, our additional type coverage may compensate for that. It will be our decision when we publish, and it will likely be before types test returns no errors.

2. We're aligning with DT but does that even make sense? For example, using a namespace here; is that beneficial over using modules? Are there other considerations that would be beneficial that are not in DT?

[dplewis]

We also said that once we achieve a decent type coverage, specifically replace enough of these any placeholders with correct types, we'll publish our types.

How do we check type overage? That's what the types test suite is for. It makes no sense to throw away years of work that went into @types/parse. It's more the just find any and replace them. For exampleParse.Cloud.beforeSave has no typing because it doesn't exist in this SDK. How do I know this because the test fails.

I doubt we'll ever align 100% with DT

The hard part is done with this PR. I have another branch that fixed over 200 errors. I can get this done in a week.

We're aligning with DT but does that even make sense? For example, using a namespace here; is that beneficial over using modules? Are there other considerations that would be beneficial that are not in DT?

Let's take Parse.Object for example the Value uses ParseObject but the Type uses Parse namespace in index.d.ts because of the dot notation. Look at the PHP SDK it uses namespaces. This SDK only uses namespaces for Types.

Unfortunately we can't rename ParseObject to Object or ParseError to Error as these classes exist in CommonJS otherwise we could fix Replace the use of 'require' by 'import' to reduce library side and improve performance. (Tree shaking), refactor the namespace exports to use export * from './Parse' or import { Query, Object } from 'parse' but that's a different story

[mtrezza]

How do we check type overage?

Right, how do we do that at all? We can't rely on DT types, because that may not be up to date. Our code is the source of truth.

For exampleParse.Cloud.beforeSave has no typing because it doesn't exist in this SDK. How do I know this because the test fails.

I guess it exists in Parse Server then? So, Parse Server should provide these specific types, not this SDK, right?

If it helps with the conversion to keep DT for now, let's keep it. But I think we should set a point in time to remove it.

[dplewis]

Right, how do we do that at all? We can't rely on DT types, because that may not be up to date. I would say

We use it as a base, we can always change and update the test suite to match the current state of the SDK.

And what is the solution if it doesn't exist in this SDK?

Just mock them and throw an error if called client side.

[mtrezza]

We use it as a base, we can always change and update the test suite to match the current state of the SDK.

OK, but let's aim to remove the DT dependency at some point in the near future.

Just mock them and throw an error if called client side.

Can't Parse Server provide these types? If the types a specific to the Server environment, the client SDK shouldn't provide them at all IMO.

[dplewis]

OK, but let's aim to remove the DT dependency at some point in the near future.

We don't have a dependency on DT, we are just using the test suite thats all

Can't Parse Server provide these types? If the types a specific to the Server environment, the client SDK shouldn't provide them at all IMO.

I don't think it can. Let's fix the types first then worry about that when we get there.

[mtrezza]

We don't have a dependency on DT, we are just using the test suite thats all

I'm referring to the dependency dtslint which we'll remove in #2435.

When it reaches 0 is when we can publish types

So we'll merge #2435 once it reaches 0, is that the plan?

[dplewis]

So we'll merge #2435 once it reaches 0, is that the plan?

Now that I think about it we can use eslint to run the tests without waiting for it to reach 0. But yes that's the plan.

[mtrezza]

You mean we could replace dtslint with eslint immediately? Do you want to do that, or should I merge the PR as is?

[dplewis]

Let's merge this first