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

Bug: "classical" custom js will not work in 6.x. #7438

Open
smpn2 opened this issue Feb 28, 2023 · 4 comments
Open

Bug: "classical" custom js will not work in 6.x. #7438

smpn2 opened this issue Feb 28, 2023 · 4 comments

Comments

@smpn2
Copy link

smpn2 commented Feb 28, 2023

Environment

Host

item version
OS
GROWI 6.0.7
node.js 16.19.1
npm 8.19.3
yarn 1.22.19
Using Docker yes
Using growi-docker-compose yes

Client

item version
OS Windows 11 Pro x64 Build 22621.1265
browser Google Chrome 110.0.5481.178 (Official Build) (64-bit) (cohort: Stable Installs & Version Pins)

How to reproduce? (再現手順)

This is a sample case.

  1. For example, insert the following code into a Wiki page
<div id="play-song-url"></div>
<pre id="play-song-lyric">
00:00 foo
00:01 bar
</pre>
  1. Then, insert the following script in your custom script
window.addEventListener('DOMContentLoaded', async () => {
    const videoElem = document.getElementById('play-song-url');
    // ... Add custom player element. (for example, adding, changing, or deleting the DOM)

    const subsElem = document.getElementById('play-song-lyric');
    // ... Overlay lyrics on custom player and parse lyrics.
});

What happens? (症状)

Changing the DOM too fast will result in hydration errors.
https://reactjs.org/docs/error-decoder.html/?invariant=418

What is the expected result? (期待される動作)

  • Next.js (SSR) hydration errors caused by custom scripts differ from previous expectations.
  • I expect it to work as it has in the past (5.x).

Note

Of course I know that the "mdcont-" prefix will be given, but unlike the expected behavior so far, it is not clearly documented.

@smpn2 smpn2 added the type/bug label Feb 28, 2023
@yuki-takei
Copy link
Member

yuki-takei commented Mar 10, 2023

@smpn2 Thank you for your report.

Of course I know that the "mdcont-" prefix will be given, but unlike the expected behavior so far, it is not clearly documented.

You're right.

"mdcont-" is added to avoid DOM clobbering, but I'm also aware that some users have pointed out another problem(e.g. the footnote links do not work by this).
So I'm considering turning off the anchor prefix. Anyway, please give me some time to think about it.

@yuki-takei
Copy link
Member

An off-topic fix, but the mdcont prefix will be removed in v6.1.0.
#7627

@tthogehoge
Copy link

It seems that it is no longer loaded with page transitions in v6.
As far as I have tried, it seems that MutationObserver monitoring is required.

window.addEventListener('load',  function() {
  // add code here

  var div = document.getElementsByClassName("dynamic-layout-root")[0];
  var mo = new MutationObserver(function() {
    // add code here
  });
  var config = {
    childList: true,
    subtree: true
  };
  mo.observe(div, config);
});

@smpn2
Copy link
Author

smpn2 commented Jan 8, 2024

As @tthogehoge said, the only way to hack it at this point is to use the Mutation Observer.

Of course, I'm not denying the technology similar to Next.js' SSR mode, but as long as it supports custom scripts, I think simple server-side rendering, which is old-fashioned and works reliably, is the best. I also think the problem is that this issue is not mentioned on the admin page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants