solid / notifications

Solid Notifications Technical Reports
https://solid.github.io/notifications/protocol
MIT License
11 stars 7 forks source link

Documents including script from dokie.li fail to load. #44

Closed elf-pavlik closed 2 years ago

elf-pavlik commented 2 years ago

I think we should serve that .js form github so it is more reliable.

I had the same issue a while ago and mentioned it during one of the earlier meetings.

Screen Shot 2022-03-16 at 18 57 33

@csarven

related: https://github.com/solid/notifications-panel/issues/36

csarven commented 2 years ago

Before putting a solution in place, can you clarify what causes the issue you're experiencing? You're the first and only one raising this - not to exclude the possibility that others haven't had this issue. It seems like your user agent is not fetching the resource / blocking.

elf-pavlik commented 2 years ago

I don't think my browser is blocking anything. That script from dokie.li usually loads just fine but at times it randomly fails to load. It just looks like dokie.li doesn't work in a reliable manner. Do you have some kind of CDN in front of it?

Have you considered making it async:

<script src="https://dokie.li/scripts/dokieli.js" async></script>

As suggested in https://github.com/solid/notifications-panel/issues/36

elf-pavlik commented 2 years ago

I also have just noticed that dokieli.js has a whooping 3.2MB making it 95% of the total spec payloads.

csarven commented 2 years ago

No CDN. Would you like to test the async with me sometime? I'd like to understand your issue better. I'd be grateful if you know how we can reduce the size: https://github.com/linkeddata/dokieli

elf-pavlik commented 2 years ago

@csarven please take a look at #50

I'd like to understand your issue better. I'd be grateful if you know how we can reduce the size: https://github.com/linkeddata/dokieli

I would suggest using https://chrisbateman.github.io/webpack-visualizer/ to see what gets bundled into that file.

It looks like dokieli bundles a ton of things. Which of them do you actually use for the spec?

elf-pavlik commented 2 years ago

It looks like dokieli bundles a ton of things. Which of them do you actually use for the spec?

Once again, I don't know why dokieli.js is needed for the spec. If the spec can be viewed without it, which seems to be the case. It seems that it only adds some extra features. In that case, it might be better to use dynamic imports to load dokieli.js just before those extra features are going to be used.

I run webpack --json > stats.json in dokieli repo and uploaded result to https://chrisbateman.github.io/webpack-visualizer/ resulting in:

image

It includes bunch of different RDF parsers, WYSIWYG text editor, and ton of other stuff which I don't see needed for any of the specs.

Beside that running npm intall in dokieli gave me

45 vulnerabilities (35 moderate, 10 high)

Which seems like another reason to rethink including that script in any of the specs /cc @acoburn

csarven commented 2 years ago

I look forward to the day where I can help you understand dokieli and the notion/point of dogfooding. But, today may not be that day.

Thanks for the feedback on the webpack visualizer. We've used such tools back in 2016.. but the situation is a bit more complicated than what appears on the surface - you'll have to see the dirty details...

Once again, you haven't shown how we can reproduce your error. And I'm genuinely trying to resolve it.

elf-pavlik commented 2 years ago

Can you share the screenshot of your Network tab? Are you experiencing the same error on other Web browsers?

This error isn't persistent, it just happens now and then. After #50 I will not even be noticing anymore when that script fails to load.

I look forward to the day when I can help you understand dokieli and the notion/point of dogfooding. But, today may not be that day.

Please do help me understand, what does I or anyone else who just wants to read the spec need that dokieli script for?

csarven commented 2 years ago

Closing this issue as 50 seems to help. You can also consider blocking the script or disabling JS on that domain.