[MGNLFE-118] p13n support for SSR Created: 23/Sep/21 Updated: 02/Dec/21 Resolved: 04/Oct/21 |
|
| Status: | Closed |
| Project: | Magnolia Frontend Helpers |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 1.1.0 |
| Type: | Improvement | Priority: | High |
| Reporter: | Bartosz Staryga | Assignee: | Canh Nguyen |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | VN-Implementation | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | 1.5d | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Template: |
|
||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||
| Task DoD: |
[X]*
Doc/release notes changes? Comment present?
[X]*
Downstream builds green?
[X]*
Solution information and context easily available?
[X]*
Tests
[X]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||
| Documentation update required: |
Yes
|
||||||||||||||||
| Date of First Response: | |||||||||||||||||
| Epic Link: | Headless p13n | ||||||||||||||||
| Sprint: | HL & LD 38 | ||||||||||||||||
| Story Points: | 2 | ||||||||||||||||
| Description |
|
.inEditor() will return false for SSR e.g. Next.js Maybe we could drop all iframe checks in those editor functions and really purely on query params and extend how inEditor works? (so it works for SSR (reading from request) as well as browser). |
| Comments |
| Comment by Bartosz Staryga [ 24/Sep/21 ] |
| Comment by Christopher Zimmermann [ 24/Sep/21 ] |
|
Do I get this correctly? You have a nextjs server running. You are using spa-rendering-extended module. Page template is specifying the nextjs server URL for `templateScript`. You are using the page editor to try to edit a page. But inEditor is returning false because it only works correctly in the context of a browser, not on a server. |
| Comment by Canh Nguyen [ 27/Sep/21 ] |
|
bstaryga We still need to support both edit and view modes for SSR. Am I right? If yes, I will modify inIframe function to check if global object has an extra flag for edit mode. WDYT? |
| Comment by Bartosz Staryga [ 27/Sep/21 ] |
|
czimmermann yes. And it is all fine as long as those checks are used only inside my code, where I have control over it. canh.nguyen imho ever place where you need to use such check inside our libs, just do a simple check if templateDefinitions or templateAnnotations are passed and are not empty. Since we already explicitly make client-side code to decide to fetch templateDefinitions or templateAnnotations, and both will be ONLY fetched when needed then editable page will have them ONLY when those are rendered inside edit view of preview iframe. So checking against them is more then enough |
| Comment by Canh Nguyen [ 27/Sep/21 ] |
|
When render SPA applications on the Page Editor, we expect that templateDefinitions or templateAnnotations must be always available for the x-editor to work properly. If templateDefinitions or templateAnnotations is missing, it will cause issues on the Page Editor e.g. preview mode, the right action bar will be empty. When calling mgnlRefresh, if annotations are missing, it will show multiple warning dialogs. So I think we must consider it and make changes in the next version.
|
| Comment by Bartosz Staryga [ 27/Sep/21 ] |
|
canh.nguyen rsiska is already doing changes to allow passing null/undefined templateDefinitions or templateAnnotations. `When calling mgnlRefresh` inside code we can also do if when calling mgnlRefresh and call it ONLY when templateDefinitions or templateAnnotations passed and not empty |
| Comment by Christopher Zimmermann [ 27/Sep/21 ] |
|
canh.nguyen Yes we need to support edit mode. Do I understand your suggestion above? To keep the iniframe function, but not have it check for the existance of window at all - but simply return a globally configured value? How would this value be set? Would I need two versions of the SPA then, one for editing and one for viewing? |
| Comment by Canh Nguyen [ 27/Sep/21 ] |
|
I think it's risky to remove the inIframe check now. On nodejs we have the global object that we can add something like mgnlEditMode=true, then the code could be: export const inIframe = () => { if (typeof window === 'undefined') { return typeof global === 'object' ? global.mgnlEditMode : false; } return window !== window.parent; };
|
| Comment by Christopher Zimmermann [ 27/Sep/21 ] |
|
Sounds OK to me, given that we need a quick, safe solution. I'm still wondering how the project code will set that variable in a nextjs project? But I see that Bartosz linked to this code above which looks promising that it can be dynamic. // Fetch template annotations only inside Magnolia WYSIWYG if (context.query.mgnlPreview === 'false') { I see that this `context` is provided by nextjs. (https://nextjs.org/docs/basic-features/data-fetching)
|
| Comment by Bartosz Staryga [ 30/Sep/21 ] |
|
As long as it works in next js (or any other SSR) it's perfect. |
| Comment by Canh Nguyen [ 04/Oct/21 ] |
|
Developers need to set global.mgnlInPageEditor to true in case rendering green bars for SSR. |