Skip to content
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.

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

simplify loaderIsVerified #232

Merged
merged 4 commits into from
Aug 15, 2023
Merged

simplify loaderIsVerified #232

merged 4 commits into from
Aug 15, 2023

Conversation

iambumblehead
Copy link
Owner

per review #231 (comment)

@iambumblehead iambumblehead mentioned this pull request Aug 15, 2023
@iambumblehead iambumblehead requested a review from koshic August 15, 2023 17:07
@iambumblehead
Copy link
Owner Author

the original suggestion is

let loaderIsVerifiedCache;
const loaderIsVerified = async () =>
  (loaderIsVerifiedCache ??= (await import(importUrl)).default === true);

and here is what is in the PR

const loaderIsVerified = (memo => async () => memo = memo || (
  (await import(loaderVerifyUrl)).default === true))()

I prefer the latter-version in the PR currently, but am flexible and fine with the suggested version as well

@koshic
Copy link
Collaborator

koshic commented Aug 15, 2023

Looks good for me, thank you! What about 'dedicated variable vs parameter reusing' - if it's your preferred style, let's keep it. I have more than enough possibilities to promote 'my style' (which is suitable for large projects) via linter rules on my current job )

@iambumblehead
Copy link
Owner Author

I like both. I slightly prefer the function version to encapsulate everything needed inside the same function

@iambumblehead iambumblehead merged commit 9df7b5b into master Aug 15, 2023
@iambumblehead iambumblehead deleted the simplify-loaderisverified branch August 15, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants