Skip to content

fix: Type regressions in ascending/descending fields #2919

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.

Sign up for GitHub

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

Open
swittk wants to merge 3 commits into
base: alpha
Choose a base branch
from
Open

fix: Type regressions in ascending/descending fields #2919

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d526a6
strict keys
swittk Feb 13, 2026
84343d2
added special field typing for full text search
swittk Feb 13, 2026
ef96ed2
run build:types
swittk Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/ParseQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1560,12 +1560,13 @@ class ParseQuery<T extends ParseObject = ParseObject> {

/**
* Method to sort the full text search by text score
* `$score` is a special key used only for full text search ranking.
*
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
sortByTextScore() {
this.ascending('$score');
this.select(['$score'] as any);
this.select('$score');
Comment on lines -1568 to +1569
Copy link
Member

@mtrezza mtrezza Feb 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give more context about why this changed from Array<string> to string? Parse.Query.select usually takes an array, but from the description I understand it also accepts a single string with comma separation. I think we should remove this type ambiguity and make select only accept an array of strings. If you agree we could pin the change for the next major release. We should then keep using the array type wherever possible because we'll have to revert to it.

Copy link
Contributor Author

@swittk swittk Feb 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply; there isn't really any need for this change (it could be just ['$score'] the same as before), but there's really no real behavioural difference underneath I think; internally it just calls Array.isArray(key) check before, but then it does the key.join() immediately after + does a split(',') afterwards, so internally it supports all 3 types I mentioned (single array of keys, keys as string args, keys as string with comma splitting), and even likely supports a mix-and-match of all 3 types as multiple args due to how it's implemented in the addAscending and addDescending call sites.
The typings I think what people are definitely using (at least in my team) are:

  • .addAscending('key1', 'key2') (the variadic arguments version)
  • or the .addAscending(['key1', 'key2']) (the array args version)

I don't recall seeing the comma separated string thing documented, but it's definitely working and has been there a long time. I don't think we should remove this to be honest. I do have an idea for how to make the comma-separated typed multiple keys thing work strongly typed, but I was thinking of doing a separate PR for this since it seemed like a bit of TS magic. Just making keys typed the conventional way seemed straightforward.

Copy link
Contributor Author

@swittk swittk Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtrezza sorry for the ping; do you have any suggestions to what I should edit?

Copy link

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtrezza, here are my suggestions on the two concerns raised:

1. The sortByTextScore change (['$score'] → '$score')

This change is a net improvement — it removes the as any type cast that was only needed because '$score' wasn't previously a recognized key in the select signature. Now that '$score' is explicitly typed, passing it as a plain string is both correct and cleaner. No behavioural difference exists at runtime (the method internally calls Array.isArray(key) then pushes/joins accordingly). I'd keep this change as-is.

2. Type inconsistency: ascending vs addAscending for '$score'

There is a subtle inconsistency in this PR worth addressing. ascending includes '$score' in its type constraint, but addAscending does not — even though ascending delegates to addAscending internally:

ascending<K extends keyof T['attributes'] | keyof BaseAttributes | '$score'>(...keys: (K | K[])[]): this {
    this._order = [];
    return this.addAscending.apply(this, keys); // delegates here
}

addAscending<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this {
    // '$score' not in K here

This means query.ascending('$score') is valid TypeScript, but query.addAscending('$score') is a type error, even though both work identically at runtime. The same gap exists for descending vs addDescending. I'd suggest aligning all four methods to include '$score' in this PR.

3. Broader API ambiguity (string vs string[] vs variadic)

Regarding mtrezza's suggestion to restrict select (and the sort methods) to arrays-only in a future major release — that seems reasonable as a long-term goal. For now, documenting all supported call signatures in the JSDoc and leaving the enforcement for a major release would be the least disruptive path. If you'd like, I can open a follow-up issue to track this.

🧠 Learnings used 
Learnt from: mtrezza
Repo: parse-community/Parse-SDK-JS PR: 2925
File: src/ParseFile.ts:334-334
Timestamp: 2026-02-20T13:47:49.469Z
Learning: In `src/ParseFile.ts`, the codebase uses `this._data = null` in multiple places (e.g., line 358 in saveFile path) even though `_data` is typed as `string | undefined`. This is a pre-existing pattern. Type alignment fixes should be done separately across all occurrences rather than piecemeal in feature PRs.

Copy link
Member

@mtrezza mtrezza Mar 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recall seeing the comma separated string thing documented

Then we shouldn't use it, because we may remove it. If it's a long-standing feature, then we might do it with the major version release, even though it's undocumented, just to minimize the effects. But in general, we need to get out of this multi-type hell. It complicates code, adds complexity where it's not necessary. That means more bugs, more maintenance effort.

For code examples, I suggest to use the variant we'll keep, which is the Array version, otherwise we have more work when we remove the other types in the future.

Does this help?

Copy link

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

return this;
}

Expand Down Expand Up @@ -1762,12 +1763,12 @@ class ParseQuery<T extends ParseObject = ParseObject> {

/**
* Sorts the results in ascending order by the given key.
*
* `$score` is a special key used only for full text search ranking.
* @param {(string|string[])} keys The key to order by, which is a
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
ascending(...keys: string[]): this {
ascending<K extends keyof T['attributes'] | keyof BaseAttributes | '$score'>(...keys: (K | K[])[]): this {
this._order = [];
return this.addAscending.apply(this, keys);
}
Expand All @@ -1780,7 +1781,7 @@ class ParseQuery<T extends ParseObject = ParseObject> {
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
addAscending(...keys: string[]): this {
addAscending<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this {
if (!this._order) {
this._order = [];
}
Expand All @@ -1801,7 +1802,7 @@ class ParseQuery<T extends ParseObject = ParseObject> {
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
descending(...keys: string[]): this {
descending<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K|K[])[]): this {
this._order = [];
return this.addDescending.apply(this, keys);
}
Expand All @@ -1814,7 +1815,7 @@ class ParseQuery<T extends ParseObject = ParseObject> {
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
addDescending(...keys: string[]): this {
addDescending<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this {
if (!this._order) {
this._order = [];
}
Expand Down Expand Up @@ -1926,10 +1927,12 @@ class ParseQuery<T extends ParseObject = ParseObject> {
* longer configured is not included. To return all auth data regardless of
* the provider configuration, do not select `authData`.
*
* `$score` is a special key used only for full text search ranking.
*
* @param {...string|Array<string>} keys The name(s) of the key(s) to include.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
select<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this {
select<K extends keyof T['attributes'] | keyof BaseAttributes | '$score'>(...keys: (K | K[])[]): this {
if (!this._select) {
this._select = [];
}
Expand Down
10 changes: 5 additions & 5 deletions types/ParseQuery.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ declare class ParseQuery<T extends ParseObject = ParseObject> {
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
ascending(...keys: string[]): this;
ascending<K extends keyof T['attributes'] | keyof BaseAttributes | '$score'>(...keys: (K | K[])[]): this;
/**
* Sorts the results in ascending order by the given key,
* but can also add secondary sort descriptors without overwriting _order.
Expand All @@ -743,15 +743,15 @@ declare class ParseQuery<T extends ParseObject = ParseObject> {
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
addAscending(...keys: string[]): this;
addAscending<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this;
/**
* Sorts the results in descending order by the given key.
*
* @param {(string|string[])} keys The key to order by, which is a
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
descending(...keys: string[]): this;
descending<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this;
/**
* Sorts the results in descending order by the given key,
* but can also add secondary sort descriptors without overwriting _order.
Expand All @@ -760,7 +760,7 @@ declare class ParseQuery<T extends ParseObject = ParseObject> {
* string of comma separated values, or an Array of keys, or multiple keys.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
addDescending(...keys: string[]): this;
addDescending<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this;
/**
* Sets the number of results to skip before returning any results.
* This is useful for pagination.
Expand Down Expand Up @@ -820,7 +820,7 @@ declare class ParseQuery<T extends ParseObject = ParseObject> {
* @param {...string|Array<string>} keys The name(s) of the key(s) to include.
* @returns {Parse.Query} Returns the query, so you can chain this call.
*/
select<K extends keyof T['attributes'] | keyof BaseAttributes>(...keys: (K | K[])[]): this;
select<K extends keyof T['attributes'] | keyof BaseAttributes | '$score'>(...keys: (K | K[])[]): this;
/**
* Restricts the fields of the returned Parse.Objects to all keys except the
* provided keys. Exclude takes precedence over select and include.
Expand Down
30 changes: 28 additions & 2 deletions types/tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1895,22 +1895,40 @@ function testQuery() {

// $ExpectType ParseQuery<MySubClass>
query.addAscending(['attribute1', 'attribute2', 'updatedAt']);

// $ExpectType ParseQuery<MySubClass>
query.addAscending('attribute1', 'attribute2', 'updatedAt');

// $ExpectError
query.addAscending(['attribute1', 'unexistenProp']);
// $ExpectType ParseQuery<MySubClass>
query.addAscending('createdAt');
// $ExpectType ParseQuery<MySubClass>
query.addAscending('updatedAt');
// $ExpectType ParseQuery<MySubClass>
query.addAscending('objectId');

// $ExpectType ParseQuery<MySubClass>
query.addDescending(['attribute1', 'attribute2', 'createdAt']);
// $ExpectError
query.addDescending(['attribute1', 'unexistenProp']);
// $ExpectType ParseQuery<MySubClass>
query.addDescending('createdAt');
// $ExpectType ParseQuery<MySubClass>
query.addDescending('updatedAt');
// $ExpectType ParseQuery<MySubClass>
query.addDescending('objectId');

// $ExpectType ParseQuery<MySubClass>
query.ascending(['attribute1', 'attribute2', 'objectId']);
// $ExpectError
query.ascending(['attribute1', 'nonexistentProp']);
// $ExpectType ParseQuery<MySubClass>
query.ascending('createdAt');
// $ExpectType ParseQuery<MySubClass>
query.ascending('updatedAt');
// $ExpectType ParseQuery<MySubClass>
query.ascending('objectId');
// $ExpectType ParseQuery<MySubClass> ($score only used for full text queries)
query.ascending('$score');

// $ExpectType ParseQuery<MySubClass>
query.containedBy('attribute1', ['a', 'b', 'c']);
Expand Down Expand Up @@ -1953,6 +1971,12 @@ function testQuery() {
query.descending(['attribute1', 'attribute2', 'objectId']);
// $ExpectError
query.descending(['attribute1', 'nonexistentProp']);
// $ExpectType ParseQuery<MySubClass>
query.descending('createdAt');
// $ExpectType ParseQuery<MySubClass>
query.descending('updatedAt');
// $ExpectType ParseQuery<MySubClass>
query.descending('objectId');

// $ExpectType ParseQuery<MySubClass>
query.doesNotExist('attribute1');
Expand Down Expand Up @@ -2134,6 +2158,8 @@ function testQuery() {
query.select('attribute1', 'attribute2');
// $ExpectType ParseQuery<MySubClass>
query.select(['attribute1', 'attribute2']);
// $ExpectType ParseQuery<MySubClass> ($score; only used for full text search ranking)
query.select('$score');
// $ExpectError
query.select('attribute1', 'nonexistentProp');

Expand Down
Loading