Move federal code to utils #3591
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #3591 +/- ##
==========================================
+ Coverage 96.47% 96.49% +0.02%
==========================================
Files 260 259 -1
Lines 60826 60840 +14
==========================================
+ Hits 58684 58710 +26
+ Misses 2142 2130 -12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
looks very good and clever execution on the deprecated federated.js file.
@mokimo Could you potentially create a ticket for the Yugo team though to stop importing this deprecated file so we can properly delete it?
Here's one of their tickets that I found if you want to clone - https://jira.corp.adobe.com/browse/MWPW-165786
|
@salonijain3 , @bandana147 , @sharmrj - it would be great to get your point of view as well |
| return fedsPlaceholderConfig; | ||
| }; | ||
| import { getFederatedContentRoot, getFederatedUrl, getFedsPlaceholderConfig } from './utils.js'; | ||
| // DO NOT USE THIS FILE, THIS IS HERE FOR HISTORY PURPOSES AND AVOIDING BREAKING CHANGES. |
There was a problem hiding this comment.
It might be worth having a console.warn() here to let any stray users know that they should no longer be importing this file. Maybe we could put a date on removing it entirely.
There was a problem hiding this comment.
I thought about it, however we only have one consumer which I created a ticket for. I dislike console.warn since it would end up on the end-users device as well. Not much harm if its only when someone checks the dev-tools, but nothing that we should have on prod either though
|
Reminder to set the |
There was a problem hiding this comment.
Verified , testing details https://jira.corp.adobe.com/browse/MWPW-167099
|
Skipped 3591: "Move federal code to utils" due to file "libs/blocks/global-navigation/global-navigation.js" overlap. Merging will be attempted in the next batch |
|
Skipped 3591: "Move federal code to utils" due to file "libs/blocks/global-footer/global-footer.js" overlap. Merging will be attempted in the next batch |
|
Skipped 3591: "Move federal code to utils" due to file "libs/blocks/global-navigation/global-navigation.js" overlap. Merging will be attempted in the next batch |
|
Skipped 3591: "Move federal code to utils" due to file "libs/blocks/global-navigation/utilities/utilities.js" overlap. Merging will be attempted in the next batch |
|
Skipped 3591: "Move federal code to utils" due to file "libs/blocks/global-footer/global-footer.js" overlap. Merging will be attempted in the next batch |
|
Skipped 3591: "Move federal code to utils" due to file "libs/blocks/global-navigation/global-navigation.js" overlap. Merging will be attempted in the next batch |
89bf1e9 to
5a805aa
Compare
|
Skipped merging 3591: Move federal code to utils due to failing or running checks |
Description
The due the growing usage of the federal project and thus the
federated.jsfile - we should move the code to the milo core utils as it has became a core feature.This should provide performance benefits as this can currently lead to being a render blocking request where certain key features such as
decorate.jsrequire to loadfederated.jsbefore the LCP.Implications
We seem to have one deep import adobecom/dme-partners · edsdme/blocks/partners-navigation/utilities/utilities.js based on a quick search...
We will need to keep the
federated.jsfile around to avoid breaking changes.Resolves: MWPW-166847
Test URLs: