Skip to content

Improve Acuant Global Code Comment #10247

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.

Sign up for GitHub

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

Merged
night-jellyfish merged 5 commits into from
Mar 15, 2024
Merged

Improve Acuant Global Code Comment #10247

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3be8cd3
Improve comment
charleyf Mar 13, 2024
8e6c001
changelog: Internal, In-Person Proofing, Update a comment about the a…
charleyf Mar 13, 2024
1ab73fb
Add a link to the PR that made the acuant global changes
charleyf Mar 14, 2024
b03f9aa
Rm line
Mar 14, 2024
977e602
Merge branch 'main' into charley/improve-acuant-global-code-comment
Mar 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions app/javascript/packages/document-capture/context/acuant.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import SelfieCaptureContext from './selfie-capture';
/**
* Global declarations
*/
declare let AcuantJavascriptWebSdk: AcuantJavascriptWebSdkInterface; // As of 11.7.0, this is now a global object that is not on the window object.
declare let AcuantCamera: AcuantCameraInterface;

declare global {
Expand Down Expand Up @@ -182,11 +181,11 @@ const getActualAcuantJavascriptWebSdk = (): AcuantJavascriptWebSdkInterface => {
if (window.AcuantJavascriptWebSdk && typeof window.AcuantJavascriptWebSdk.start === 'function') {
return window.AcuantJavascriptWebSdk;
}
if (typeof AcuantJavascriptWebSdk === 'undefined') {
if (!window.AcuantJavascriptWebSdk) {
// eslint-disable-next-line no-console
console.error('AcuantJavascriptWebSdk is not defined in the global scope');
Comment on lines 185 to 186
Copy link
Copy Markdown
Contributor

@aduth aduth Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in scope of what you'd aimed to tackle, but I wonder why we log a message here. If we wanted to log something for us to be aware of, I'd think we'd use trackError. Or if it's for tests (to trip the browser log detection), I'd think we could wrap that in process.env.NODE_ENV. If it's for local development, I wouldn't think we should have it part of the committed code.

Copy link
Copy Markdown
Contributor

@night-jellyfish night-jellyfish Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. I'm not sure what the original intent of this was either, tbh. It was added almost 2 years ago and I don't see a direct reference to the "why" for the console error.

I think reading through it, my take on it would be that we'd want to know if it's happening in production. The user would probably be encountering some kind of error already if there is no window.AcuantJavascriptWebSdk. And I think we'd want to know if that's happening to users (especially if it's happening to many).

So I think TrackError makes sense. (TIL trackError exists! I thought we always used trackEvent). I created LG-12733 to track that work. I found we use it twice in this file as well.

}
return AcuantJavascriptWebSdk;
return window.AcuantJavascriptWebSdk;
};

/**
Expand Down