chore: deprecate orderings string values, resolves #269

[lihbr]

Types of changes

• Chore (a non-breaking change which is related to package maintenance)
• Bug fix (a non-breaking change which fixes an issue)
• New feature (a non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Deprecate orderings string values and non-array values. See #269 for more details.

This is a minor breaking change for TypeScript users, while still working, type checking won't pass for deprecated values anymore, this has been reflected in the migration guide.

Resolves: #269

Checklist:

• My change requires an update to the official documentation.
• All TSDoc comments are up-to-date and new ones have been added where necessary.
• All new and existing tests are passing.

[angeloashmore]

Is it possible we could use @deprecate to mark string | Ordering as deprecated, but still accept Ordering[]? Perhaps using a union on the config type?

Otherwise, everything looks good!

[lihbr]

Is it possible we could use @deprecate to mark string | Ordering as deprecated, but still accept Ordering[]? Perhaps using a union on the config type?

I tried to do this at first yes, but no :/
Tried converting the interface to a type and extending it with a union in various ways but it either resolved to any or never :/

I fixed the conflicts, and won't merge for now in case you want to play with it yourself~ LMK

[lihbr]

Again no pressure as it's a non urgent topic, but just a gentle ping ☺️ @angeloashmore

[angeloashmore]

I updated the PR with the following changes:

Orderings

• Restored string | string[] to the orderings parameter. Since we want to mark them as deprecated, and not removed, we need to keep the types. Removing them could break existing projects.
• Added console.warn() messages that print when NODE_ENV is development and a string value is detected.
• Added a orderings-must-be-an-array-of-objects.md message used in the console.warn() calls.

Filters — These changes are outside the scope of the PR but are similar to the ones above

• Added a console.warn() message that print when NODE_ENV is development and a string value is detected.
• Added a filters-must-be-an-array.md message used in the console.warn() call.

I think that gives us the effect we want: don't break existing projects as a result of this change, but nudge developers to update their projects.

If it looks good to you, this PR is good to merge!

[codecov-commenter]

Codecov Report

Merging #279 (35b90f0) into v7 (a520aa4) will decrease coverage by 0.04%.
The diff coverage is 97.77%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##               v7     #279      +/-   ##
==========================================
- Coverage   99.96%   99.92%   -0.04%     
==========================================
  Files          48       49       +1     
  Lines        5281     5359      +78     
  Branches      303      309       +6     
==========================================
+ Hits         5279     5355      +76     
- Misses          2        4       +2     

Impacted Files	Coverage Δ
src/buildQueryURL.ts	99.46% <97.14%> (-0.54%)	⬇️
src/lib/devMsg.ts	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[lihbr]

Awesome, thanks for the update 🙏