Allow api version overrides

[paulomarg]

WHY are these changes introduced?

Currently, we receive an apiVersion setting when calling shopifyApi(), and that version is used for any clients throughout the app lifecycle. However, using different versions of the API in different places is a valid use case, for example when trying out a developer preview without migrating everything else.

With the way things work now, the clients are unnecessarily rigid and make it harder to interact with multiple versions of the API.

WHAT is this pull request doing?

Adding an apiVersion override to all of the clients' constructors, that will be used instead of the global value when present.

With this, we can also update the REST resources to internally use their own API version, thus removing the requirement for the app to load the "right" REST resources - with this change, apps can load whatever resources they want to use. We still log a warning if the version mismatches to help remind folks that they might want to update their resources when they change the main version.

Bonus: it seems we'd accidentally removed the REST resource base tests, I'm adding those back. You can ignore those, and the actual changes are here: https://github.com/Shopify/shopify-api-js/pull/660/files/2965e1a9f07ef99e50fd0647f28b17c213dc0476..ea292fbb1fdb4a20213ff9ac54a7144562f71a62

Type of change

• Minor: New feature (non-breaking change which adds functionality)

Checklist

• I have added a changelog entry, prefixed by the type of change noted above
• I have added/updated tests for this change
• I have documented new APIs/updated the documentation for modified APIs (for public APIs)

[teddyhwang]

do we do any validation that this is a valid apiVersion?

[paulomarg]

We don't usually validate this kind of param - we're trusting TS to shout if anything is wrong, but this can be a version string ("2023-04") in pure JS and it will work normally.

[cquemin]

@paulomarg since you change the argument type, I am wondering if these aren't breaking changes? It looks like it mostly on internal object but I am not 100% sure

[paulomarg]

@paulomarg since you change the argument type, I am wondering if these aren't breaking changes? It looks like it mostly on internal object but I am not 100% sure

We're mostly just adding an optional apiVersion to the client constructors, so that won't break (most of the tests still use them the "old" way). The loadRestResources function is not exported so it shouldn't be a problem either.

What just occurred to me is that the CONFIG static in the clients is technically public. Even though it's extremely unlikely apps will access those, should we consider this a breaking change?

[cquemin]

@paulomarg since you change the argument type, I am wondering if these aren't breaking changes? It looks like it mostly on internal object but I am not 100% sure

We're mostly just adding an optional apiVersion to the client constructors, so that won't break (most of the tests still use them the "old" way). The loadRestResources function is not exported so it shouldn't be a problem either.

What just occurred to me is that the CONFIG static in the clients is technically public. Even though it's extremely unlikely apps will access those, should we consider this a breaking change?

I guess potentially ? I am not sure if we want to go down this route for the few that will be impacted or not. If we apply the rules strictly then yes maybe it should be, but it means waiting for the next major release...

[paulomarg]

I agree, IMO we shouldn't hold this change back because of that.

[cquemin]

Following our conversation: you're good to go then