keyoxide/doipjs

Fork 28

Code Issues 56 Pull requests 24 Projects Releases 89 Activity

Refactor proofs module #75

Open

vladimyr wants to merge 1 commit from vladimyr/doipjs:refactor-proofs into dev

pull from: vladimyr/doipjs:refactor-proofs

merge into: keyoxide:dev

keyoxide:main

keyoxide:dev

keyoxide:fix-irc-fetcher

keyoxide:new-claim-eveonline-143

keyoxide:prepare-new-release

keyoxide:fix-jsdoc-types

keyoxide:yarn-to-npm

keyoxide:support-openpgp-aspe-claims

keyoxide:support-html-alias

keyoxide:improve-activitypub-support

keyoxide:v1-restructure

keyoxide:into-es-module

keyoxide:support-aspe

keyoxide:fix-js-lsp-issues

keyoxide:improve-linting

keyoxide:add-markers

keyoxide:support-opencollective-claim

keyoxide:support-entity-decoding

keyoxide:use-rome-tools

keyoxide:support-cloudflare-buster

keyoxide:support-fediverse-posts

keyoxide:matrix-room-verification

Conversation 5 Commits 1 Files changed 1 +37 -67

vladimyr commented

2024年01月30日 04:10:07 +01:00

Member

Copy link

There isn't much to say about this except that I got rid of nested Promise-s by leveraging async-await 🤷

There isn't much to say about this except that I got rid of nested `Promise`-s by leveraging async-await 🤷

vladimyr added the

labels

2024年01月30日 04:10:26 +01:00

vladimyr referenced this pull request

2024年01月30日 09:37:39 +01:00

WIP: Layering custom protocol fetchers on top of a generic HTTP one #76

yarmo commented

2024年01月30日 11:08:31 +01:00

Owner

Copy link

Looks good to me. I remember when I wrote this a few years ago, the use of async in that particular spot was clashing with some linting rules and thus I resorted to that promise situation.

Glad it's all good now and feels much less clunky.

I never really did like the logic I used to find out whether to make a direct request or proxy request. But that's outside the scope of this PR.

I'm happy with this

Looks good to me. I remember when I wrote this a few years ago, the use of async in that particular spot was clashing with some linting rules and thus I resorted to that promise situation. Glad it's all good now and feels much less clunky. I never really did like the logic I used to find out whether to make a direct request or proxy request. But that's outside the scope of this PR. I'm happy with this

yarmo approved these changes

2024年01月30日 11:08:50 +01:00

Ryuno-Ki reviewed

2024年01月30日 18:05:49 +01:00

src/proofs.js Outdated

@ -194,0 +162,4 @@

try {

return createDefaultRequestPromise(data, opts)

} catch {

return createProxyRequestPromise(data, opts)

Ryuno-Ki commented

2024年01月30日 18:05:49 +01:00

Owner

Copy link

If it works: great!

This is the only section of the PR, which I couldn't tell off-hand whether it is equivalent to the previous implementation.

If it works: great! This is the only section of the PR, which I couldn't tell off-hand whether it is equivalent to the previous implementation.

vladimyr commented

2024年01月31日 02:13:35 +01:00

Author

Member

Copy link

Glad that you called that out cause it is indeed wrong. Won't work without adding await-s:

try {
 return await createDefaultRequestPromise(data, opts)
} catch {
 return await createProxyRequestPromise(data, opts)
}

But at this point becomes rather ugly and it is probably better to use standard Promise form:

return createDefaultRequestPromise(data, opts)
 .catch(() => createProxyRequestPromise(data, opts))

Glad that you called that out cause it is indeed wrong. Won't work without adding `await`-s: ```js try { return await createDefaultRequestPromise(data, opts) } catch { return await createProxyRequestPromise(data, opts) } ``` But at this point becomes rather ugly and it is probably better to use standard Promise form: ```js return createDefaultRequestPromise(data, opts) .catch(() => createProxyRequestPromise(data, opts)) ```

👍 1

Ryuno-Ki commented

2024年02月01日 22:55:48 +01:00

Owner

Copy link

Oh, so await vs. return vs. return await is still valid?

Oh, so [await vs. return vs. return await](https://jakearchibald.com/2017/await-vs-return-vs-return-await/) is still valid?

❤️ 1

vladimyr commented

2024年02月02日 13:40:23 +01:00

Author

Member