Conversation
| } | ||
|
|
||
| // will be usefull when all data are retrieved async | ||
| async _initialize(){ |
There was a problem hiding this comment.
Can you explain why this will be useful?
What is difference between the constructor and the initialise function?
There was a problem hiding this comment.
In the future, all data may be retrieved asynchronously. Just setting things up.
| } | ||
|
|
||
| showConsentTool() { | ||
| async showConsentTool() { |
There was a problem hiding this comment.
Can you please explain the changes here?
There was a problem hiding this comment.
the async is not really important, i just forgot to delete this change.
The real changes are the ones below this line.
|
|
||
| export default function initCmp(loaderData) { | ||
| export default async function initCmp(loaderData) { | ||
|
|
There was a problem hiding this comment.
Again please can you explain the benefits of the suggested approach?
There was a problem hiding this comment.
Better readability. Assuming we know how async/await works
| const isCookiePresent = (typeof cookie === 'string'); | ||
| console.log('[INFO][Module-isShowUi]: ', !isCookiePresent); | ||
| return Promise.resolve(!isCookiePresent); | ||
| // make it sync |
There was a problem hiding this comment.
If we make it sync, it cannot be used in a promise chain right?
There was a problem hiding this comment.
yes.. but by nature, it is not async. so we dont have to make it return promise.
edit: actually, we can use it in promise chain. but not on the start of chain.
|
|
||
| // prevent this script from running without window object | ||
| // e.g: SSR | ||
| if(typeof window === 'undefined') return; |
There was a problem hiding this comment.
But we need to try init() again?
There was a problem hiding this comment.
yes, in the browser.
For certain library (anything with node.js + SSR), Javascript are run in the server. So if this is run in the server, there will be no ERROR saying: cannot find {property} of undefined, or something like that
| .catch(err => console.error(err)); | ||
|
|
||
| await initApi(cmp); | ||
| const result = isShowUi(loaderData.iabCookie) ? await cmp.showConsentTool() : true; |
There was a problem hiding this comment.
This feels less readable, can you explain the benefit?
| const result = isShowUi(loaderData.iabCookie) ? await cmp.showConsentTool() : true; | ||
|
|
||
| // why do we need window.__cpm ? | ||
| cmp.readyCmpAPI(result); |
There was a problem hiding this comment.
I agree window is not good to attach to, but it is to stop having to import the cmp object in all files, maybe it should be created and exported elsewhere?
2 participants
My reviews are mostly about promise, and the use of async.
Other than that:
windowdoesn't existswindow.cmptagManagerModule