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

smpn2 commented 1 year ago

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.

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>

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.

yuki-takei commented 1 year ago

@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, ｐlease give me some time to think about it.

yuki-takei commented 1 year ago

An off-topic fix, but the mdcont prefix will be removed in v6.1.0. https://github.com/weseek/growi/pull/7627

tthogehoge commented 1 year ago

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 commented 8 months ago

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.

weseek / growi