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

[anything] Init not from 'ready', but right away -> compatible with Promise-based plugins #154

Open
peter-lyons-kehl opened this issue Oct 1, 2022 · 1 comment

Comments

@peter-lyons-kehl
Copy link

peter-lyons-kehl commented Oct 1, 2022

Hi Asvin,

Thank you for anything. It's a hidden gem.

Side note/FYI: Its description on Reveal.js's wiki didn't do it justice, so I updated it.

Side note: My use case:

  1. I'm working on reusable JS to help with Reveal.js & Markdown (file-based) presentations & embedding source code from files (rather than included in HTML/Markdown) that would work either on GitHub Pages or through another web server. There are a few code embedding plugins. The most up-to-date & advanced is https://github.com/befocken/revealjs-embed-code.

  2. After loading Markdown (from files), I use anything to update data-url of <code> for revealjs-embed-code. However, revealjs-embed-code doesn't use Reveal.js's ready event. Instead, its init() calls document.querySelectorAll() straightaway, and it returns a Promise. That Promise gets completed before anything gets triggered by Reveal.js's ready event.

About Reveal.js and plugin initialization:

  1. https://revealjs.com/creating-plugins (bold formatting is mine; 'Note:...' parts are mine):
  • "init An optional function that is called when the plugin should run..." - Note: That sounds like the primary place to initialize a plugin.
  • "The init function can optionally return a Promise. If a Promise is returned, reveal.js will wait for it to resolve before the presentation finishes initialization and fires the ready event." - Note: Any Promise-based plugins won't be able to co-operate with anything (as-is).
  1. https://revealjs.com/events/#ready: "The ready event is fired when reveal.js has loaded all non-async dependencies and is ready to accept API calls."
  • Since revealjs-embed-code uses async, and async-compatibility is the way (for smoothness/responsiveness), it seems to me that Reveal.js's ready event is not appropriate for plugin initialization.
  • init() in anything doesn't use any Reveal.js API calls, so it doesn't need to be invoked from ready.

The fix is to remove the ready event attachment, and process the same directly from init()/initAnything(): peter-lyons-kehl@5213b13. However, that is a breakable change if anyone passed RevealAnything to Reveal.initialize() before plugin(s) that need to process content before RevealAnything.

// Works with RevealAnything as-is, but would be broken with the proposed change:
plugins: [
    RevealAnything,
    RevealMarkdown, // loads embedded content and/or file(s) that contains class (& optional HTML comments) handled by RevealAnything
]

// Works with RevealAnything both as-is and with the proposed change:
plugins: [
    RevealMarkdown, // loads embedded content and/or file(s) that contains class (& optional HTML comments) handled by RevealAnything
    RevealAnything
]

There is a way to make the change non-breakable: To add an (optional) intitialization field for anything which would activate/switch to the new behavior, something like:

anything: [
                  {
                     init_in_listed_plugin_order: true
                  },
					{
						className: "presentation_github_repo_blob_relative_link",
						initialize: make_blob_link_relative_to_presentation_github_repo
					},
          ]

But that seems convoluted/hacking. Also, based on my search (it took me very long to locate anything - hence I updated Reveal.js's wiki as per above), it seems that no one (or very few people), are using anything in such complex scenarios. (That's unfortunate, because it helps so much.) And if they do, they load it from their local copy anyway. Plus, those people are advanced users, so if they download/git clone/git pull this update, they are likely to test or read your release notes. Hence, suggest the breaking change.

Thank you for considering.

My use case (work in progress, may break)

@peter-lyons-kehl peter-lyons-kehl changed the title [anything] [anything] Init not from 'ready', but right away -> compatible with Promise-based plugins Oct 2, 2022
@rajgoel
Copy link
Owner

rajgoel commented Oct 4, 2023

Peter, thanks a lot for the feedback.

Plugin dependencies are not straight forward, see hakimel/reveal.js#3397 and have changed since I originally created anything. My mindset in create anything was about already loaded content. You use case seems different and looks interesting.

Unfortunately, I cannot invest much time, but would be happy for any PR modernizing the plugin. It would be great to use promises. Also I think it is an acceptable breaking change to require the user to load plugins in a suitable order.

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

No branches or pull requests

2 participants