Skip to content
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.

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

NciDsJsInit component outputs escaped html instead of script when publishing to doc site. #1497

Closed
bryanpizzillo opened this issue Jan 25, 2024 · 3 comments · Fixed by #1502 or #1541
Closed
Assignees

Comments

@bryanpizzillo
Copy link
Member

Issue description

ESTIMATE TBD

Using the Header component page as an example of this not working correctly, but:

<NciDsJsInit>{`
const elements = preview.querySelectorAll('.nci-header');
elements.forEach((element) => {
	switch (element.id) {
		case 'default-header-example':
			window.ncids.NCIExtendedHeaderWithMegaMenu.create(element, {
				megaMenuSource: new window.ncids.DefaultMegaMenuSource(),
				mobileMenuSource: new window.ncids.DefaultMobileMenuSource(),
			});
			break;
		case 'header-with-mega-menu-example':
			window.ncids.NCIExtendedHeaderWithMegaMenu.create(element, {
				megaMenuSource: window.adapter,
				mobileMenuSource: new window.ncids.DefaultMobileMenuSource(),
			});
			break;
	}
});
`}</NciDsJsInit>

Is being generated as:

<div>
   const elements = preview.querySelectorAll(&#x27;.nci-header&#x27;);
   elements.forEach((element) =&gt; {
   switch (element.id) {
   case &#x27;default-header-example&#x27;:
   window.ncids.NCIExtendedHeaderWithMegaMenu.create(element, {
   megaMenuSource: new window.ncids.DefaultMegaMenuSource(),
   mobileMenuSource: new window.ncids.DefaultMobileMenuSource(),
   });
   break;
   case &#x27;header-with-mega-menu-example&#x27;:
   window.ncids.NCIExtendedHeaderWithMegaMenu.create(element, {
   megaMenuSource: window.adapter,
   mobileMenuSource: new window.ncids.DefaultMobileMenuSource(),
   });
   break;
   }
   });
   </div>

It should be:

<script>
const elements = preview.querySelectorAll('.nci-header');
elements.forEach((element) => {
	switch (element.id) {
		case 'default-header-example':
			window.ncids.NCIExtendedHeaderWithMegaMenu.create(element, {
				megaMenuSource: new window.ncids.DefaultMegaMenuSource(),
				mobileMenuSource: new window.ncids.DefaultMobileMenuSource(),
			});
			break;
		case 'header-with-mega-menu-example':
			window.ncids.NCIExtendedHeaderWithMegaMenu.create(element, {
				megaMenuSource: window.adapter,
				mobileMenuSource: new window.ncids.DefaultMobileMenuSource(),
			});
			break;
	}
});
</script>
@bryanpizzillo bryanpizzillo changed the title NciDsJsInit component outputs escaped html instead of script. NciDsJsInit component outputs escaped html instead of script when publishing to doc site. Jan 25, 2024
@bryanpizzillo
Copy link
Member Author

This maybe related, but the error:

warning Component NciDsJsInit was not imported, exported, or provided by MDXProvider as global scope                                                                                            
warning Component NciDsJsInit was not imported, exported, or provided by MDXProvider as global scope                                                                                            
warning Component NciDsJsInit was not imported, exported, or provided by MDXProvider as global scope  

Appears in the build logs. Additionally there are 3 mdx pages under docs/content/components that have <NciDsJsInit>. 3 errors, 3 files. Probably not a coincident.

Is appearing in the build output.

dlescarbeau added a commit that referenced this issue Jan 30, 2024
dlescarbeau added a commit that referenced this issue Jan 30, 2024
dlescarbeau added a commit that referenced this issue Jan 30, 2024
dlescarbeau added a commit that referenced this issue Jan 31, 2024
dlescarbeau added a commit that referenced this issue Jan 31, 2024
dlescarbeau added a commit that referenced this issue Jan 31, 2024
@bennettcc bennettcc added the Needs Technical Review Needs review from Architect and Development label Feb 1, 2024
dlescarbeau added a commit that referenced this issue Feb 5, 2024
dlescarbeau added a commit that referenced this issue Feb 5, 2024
dlescarbeau added a commit that referenced this issue Feb 5, 2024
bryanpizzillo pushed a commit that referenced this issue Feb 5, 2024
@bryanpizzillo
Copy link
Member Author

The fix only stopped errors. The NCI Header with Mega Menu example does not initialize.
It appears that it is related to the code in NciDsJsInit.jsx:

...SNIP...
	window.ncidsPreviewHandlers = window.ncidsPreviewHandlers || {};
	  if (!window.ncidsPreviewHandlers['${path}']) {
			const handler = (preview) => {
				${children}
		  }
	  }
...SNIP...

We never actually add the created handlers to window.ncidsPreviewHandlers.

This should probably change to:

...SNIP...
	window.ncidsPreviewHandlers = window.ncidsPreviewHandlers || {};
	  if (!window.ncidsPreviewHandlers['${path}']) {
			window.ncidsPreviewHandlers['${path}'] = (preview) => {
				${children}
		  }
	  }
...SNIP...

@bryanpizzillo bryanpizzillo reopened this Feb 5, 2024
@bennettcc bennettcc added Fix per Feedback and removed Needs Technical Review Needs review from Architect and Development labels Feb 6, 2024
dlescarbeau added a commit that referenced this issue Feb 6, 2024
dlescarbeau added a commit that referenced this issue Feb 6, 2024
dlescarbeau added a commit that referenced this issue Feb 6, 2024
dlescarbeau added a commit that referenced this issue Feb 6, 2024
dlescarbeau added a commit that referenced this issue Feb 6, 2024
@dlescarbeau
Copy link
Collaborator

dlescarbeau commented Feb 6, 2024

This issue remains open / on hold. There is a PR raised that worked to address this, but it ran into problems related to Gatsby.

The Javascript required by the components' Previews is not called on first page load due to how Gatsby handles the page rendering. This also includes navigating to the page via our Side Navigation or other link on the site due to how Gatsby's Link component preload's the destination page's resources.

Refreshing the page allows the javascript to fire and therefore the Javascript needed by the Preview is able to work appropriately (for example, the initialization of the Header Mega Menu or Site Alert). This is consistent across pages with Javascript required by their component previews.

Without a refresh, the first visit to these documentation pages will display the previewed components without working javascript.

bryanpizzillo added a commit that referenced this issue Feb 20, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

Closes #1497
bryanpizzillo added a commit that referenced this issue Feb 21, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497

.
bryanpizzillo added a commit that referenced this issue Feb 21, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497

.
bryanpizzillo added a commit that referenced this issue Feb 21, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497

.
bryanpizzillo added a commit that referenced this issue Feb 21, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497
bryanpizzillo added a commit that referenced this issue Feb 21, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497
olitharp-nci pushed a commit that referenced this issue Feb 22, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497
bryanpizzillo added a commit that referenced this issue Feb 22, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497
olitharp-nci pushed a commit that referenced this issue Feb 22, 2024
So TL;DR; Gatsby v3 does not make a static HTML site. It makes some sort
of Static/React mess. Add pre-fetching on top of this and we start running
into issues with components that need to get added to the DOM happening
randomly. This hopefully resolves some of our current issues.

See https://github.com/NCIOCPL/ncids/wiki/Technical:-NCIDS-Initialization-in-Gatsby

Closes #1497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants