MWPW-157555: Remove useless logic and putting import commerce.js top. #2906
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2906 +/- ##
==========================================
+ Coverage 96.24% 98.21% +1.97%
==========================================
Files 236 67 -169
Lines 54233 8074 -46159
==========================================
- Hits 52196 7930 -44266
+ Misses 2037 144 -1893 ☔ View full report in Codecov by Sentry. |
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
seanchoi-dev
changed the title
seanchoi-dev
changed the title
seanchoi-dev
changed the title
| @@ -2,6 +2,7 @@ import { | |||
| createTag, getConfig, loadArea, loadScript, loadStyle, localizeLink, | |||
| } from '../../utils/utils.js'; | |||
| import { replaceKey } from '../../features/placeholders.js'; | |||
| import '../../deps/mas/commerce.js'; | |||
There was a problem hiding this comment.
Is this needed? This is also being imported in L427
There was a problem hiding this comment.
Yes, we want to start to load this ASAP when merch.js get loaded.
This is what task's description says:
"Remove the related logic and move import of commerce.js to the top."
| commerceLibPath = `${getMasBase(hostname, maslibs)}/libs/commerce.js`; | ||
| } | ||
| } | ||
| const commerceLibPath = '../../deps/mas/commerce.js'; |
There was a problem hiding this comment.
I think line and next one is not needed anymore.
There was a problem hiding this comment.
After remove those, how do you want to define commerceLib.init() from next line?
There was a problem hiding this comment.
moving it to line 5 should work no?
There was a problem hiding this comment.
Maybe I am not getting it but, my question is,
The function scope goes like initService() -> initService.promise.then() -> commerceLib.init().
And the commerceLib is defined relying on importing ('../../deps/mas/commerce.js')
I am not sure how we want to define commerceLib.
Are you saying I should just put const commerceLib = await import('../../deps/mas/commerce.js'); on top instead import '../../deps/mas/commerce.js'; ?
|
@seanchoi-dev the following test should be removed.
|
I removed it. |
Resolves: MWPW-157555
Test URLs: