Disable unnecessary and heavy downcompilation of async/await syntax

[patdx]

Issue summary

The stack traces for shopify API are a bit on the obtuse side, for example I just ran into this:

• @shopify/shopify-api version: 6.0.2
• Node version: 18
• Operating system: Mac OS

Error: Cannot complete OAuth process. Could not find an OAuth cookie for shop url: example.myshopify.com
    at CookieNotFound2.ShopifyError2 [as constructor] (worker.mjs:22123:28)
    at new CookieNotFound2 (worker.mjs:22284:42)
    at Object.<anonymous> (worker.mjs:31902:25)
    at step (worker.mjs:21866:25)
    at Object.next (worker.mjs:21813:20)
    at fulfilled (worker.mjs:21784:30)

This is kind of compounded by the fact that some issues must be debugged in a production configuration where source maps may not be available.

We can see that the source async await is downcompiled into generator syntax:

https://unpkg.com/browse/@shopify/shopify-api@6.0.2/lib/auth/oauth/oauth.js

Expected behavior

See if it is actually necessary to downcompile the async/await syntax in the NPM library?

If it can be preserved as is, this will give more useful stack traces.

Async/await has been supported in Node since v8.

The oldest version of Node that is still in maintenance is v14.

Additionally, is is quite easy these days for the end user to downcompile async/await syntax for a runtime that does not support it.

[github-actions]

This issue is stale because it has been open for 90 days with no activity. It will be closed if no further action occurs in 14 days.

[patdx]

Still relevant with the latest version of this library: https://unpkg.com/browse/@shopify/shopify-api@6.2.0/lib/auth/oauth/oauth.js

[github-actions]

This issue is stale because it has been open for 90 days with no activity. It will be closed if no further action occurs in 14 days.

[patdx]

This is still relevant with the latest version of this library:

https://unpkg.com/browse/@shopify/shopify-api@7.0.0/lib/auth/oauth/oauth.js

Note that Node v14 is also end of life as today, the lowest version of Node that is still actively maintained is v16, which definitely has async/await support. like every version of Node since v8. Could you please review your build settings and see if it is really still necessary to downcompile the async functions?

[github-actions]

This issue is stale because it has been open for 90 days with no activity. It will be closed if no further action occurs in 14 days.

[patdx]

Still relevant: https://unpkg.com/browse/@shopify/shopify-api@7.4.0/lib/auth/oauth/oauth.js

[patdx]

I think that every platform that Shopify supports also supports async/await syntax natively. So I think the transpilation does not bring any benefits and has several downsides:

• Heavier bundle size
• Slower code
• Harder to read and debug
• Dependency on tslib

[public]

I would like to add that using a pre async/await target results in stack traces that are more than obtuse, they frequently omit the actual code that called into the shopify library in the first place making debugging which code is having problems next to impossible without wrapping everything in your own try/catch blocks.

Even if you do want to wrap everything in try/catch, in a large code base finding them all can be challenging when the stack doesn't tell you which code actually made the call that went wrong.

[github-actions]

This issue is stale because it has been open for 90 days with no activity. It will be closed if no further action occurs in 14 days.

[patdx]

I agree completely with @public's comment above: #657 (comment)

The latest version of shopify-api package on NPM still has this issue: https://unpkg.com/browse/@shopify/shopify-api@8.0.1/lib/auth/oauth/oauth.js

[github-actions]

This issue is stale because it has been open for 90 days with no activity. It will be closed if no further action occurs in 14 days.

[patdx]

Not stale, still an issue in the latest version:

https://unpkg.com/browse/@shopify/shopify-api@8.1.1/lib/auth/oauth/oauth.js

The compiled npm code looks like:

function validQuery({ config, query, stateFromCookie, }) {
    return tslib_1.__awaiter(this, void 0, void 0, function* () {
        return ((yield (0, hmac_validator_1.validateHmac)(config)(query)) &&
            (0, safe_compare_1.safeCompare)(query.state, stateFromCookie));
    });
}

While, as far as I am aware, any runtime Shopify supports could handle the native async syntax in the original source:

async function validQuery({
  config,
  query,
  stateFromCookie,
}: {
  config: ConfigInterface;
  query: AuthQuery;
  stateFromCookie: string;
}): Promise<boolean> {
  return (
    (await validateHmac(config)(query)) &&
    safeCompare(query.state!, stateFromCookie)
  );
}

Using Shopify APIs has plenty of challenges for regular devs as it is. If this this compilation setting is changed you can get the following benefits:

• reduce the barrier of entry for new Shopify devs (with more useful error stack traces)
• reduce size of the code
• Improve performance of the code (using native async instead of a polyfill)

[github-actions]

We're labeling this issue as stale because there hasn't been any activity on it for 60 days. While the issue will stay open and we hope to resolve it, this helps us prioritize community requests.

You can add a comment to remove the label if it's still relevant, and we can re-evaluate it.

[paulomarg]

Hey folks, thanks for your patience. As part of merging our repos / revamping the build process, we've also updated our TS target so hopefully this will be improved. Please let us know in https://github.com/Shopify/shopify-app-js if you still run into problems!