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

ReferenceError: navigator is not defined using Markdown plugin API in node.js #3517

Closed
MartenBE opened this issue Nov 3, 2023 · 10 comments
Closed
Labels

Comments

@MartenBE
Copy link

MartenBE commented Nov 3, 2023

When executing the following code in a node.js application:

import Reveal from 'reveal.js/dist/reveal.js';
import Markdown from 'reveal.js/plugin/markdown/markdown.js';
Reveal.initialize({ plugins: [Markdown, Highlight] });

We get the following error:

ReferenceError: navigator is not defined
    at /home/martijn/git/reveal-md/node_modules/reveal.js/dist/reveal.js:8:1705
    at /home/martijn/git/reveal-md/node_modules/reveal.js/dist/reveal.js:8:84
    at Object.<anonymous> (/home/martijn/git/reveal-md/node_modules/reveal.js/dist/reveal.js:8:201)
    at Module._compile (node:internal/modules/cjs/loader:1241:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12)
    at cjsLoader (node:internal/modules/esm/translators:283:17)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:233:7)
    at ModuleJob.run (node:internal/modules/esm/module_job:217:25)

We would like to call the slidify function from the Markdown plugin manually. How do we best go about it?

@MartenBE
Copy link
Author

MartenBE commented Nov 3, 2023

If we use the .esm.js files as mentioned in the documentation, we get the following:

import Reveal from 'reveal.js/dist/reveal.esm.js';
import Markdown from 'reveal.js/plugin/markdown/markdown.esm.js';
Reveal.initialize({ plugins: [Markdown, Highlight] });
(node:111874) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node-20 --trace-warnings ...` to show where the warning was created)
/home/martijn/git/reveal-md/node_modules/reveal.js/dist/reveal.esm.js:8
const e=(e,t)=>{for(let i in t)e[i]=t[i];return e},t=(e,t)=>Array.from(e.querySelectorAll(t)),i=(e,t,i)=> # ...                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                 

SyntaxError: Unexpected token 'export'
    at internalCompileFunction (node:internal/vm:73:18)
    at wrapSafe (node:internal/modules/cjs/loader:1153:20)
    at Module._compile (node:internal/modules/cjs/loader:1205:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12)
    at cjsLoader (node:internal/modules/esm/translators:283:17)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:233:7)
    at ModuleJob.run (node:internal/modules/esm/module_job:217:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:308:24)

@t-fritsch
Copy link
Collaborator

Hello !

Reveal.js is not intended to run server side (ie. in node) : since it is a frontend library it relies heavily on browser-ony API (window, navigator, etc.) that node doesn't have.

I'm closing this issue (related to #2878 and #3417) but feel free to comment if you think it should be reopened.

@MartenBE
Copy link
Author

MartenBE commented Nov 3, 2023

@t-fritsch This issue has been opened due to a breaking change in the Markdown plugin going from 4.5.0 to 4.6.1 as mentioned here: webpro/reveal-md#462 . This plugin is used in Reveal-md.

The call to the Markdown plugin happens at https://github.com/webpro/reveal-md/blob/be9dd1154e3e270c1e6b699c52901982d37a822b/lib/util.js#L15 , but due to the adding of this line in Reveal.js: 31174cb#diff-798667ccd1007ded74fcdc410627add5ca0946dd54e4b5e0a22a5559900fb97aR98 , we cannot use the call to the plugin no more as getConfig is undefined for us. Do you have any idea then to mitigate this problem?

@t-fritsch
Copy link
Collaborator

t-fritsch commented Nov 5, 2023

Hey, thank you for the clarification, I didn't understand the issue at first.
In fact the commit you mentioned (31174cb) adds a functionality that is important to Reveal (it's only my opinion, since I made this particular commit 😂). So reverting the commit isn't really an option.

Achieving the same functionality without accessing Reveal.getConfig() isn't possible at least in my understanding of Reveal core code (I'm quite a new contributor), and apparently it's the object returned by this method that contains references to navigator. If you see a better way to do the same thing (retrieving user-defined config) without introducing a bug in the external tool you use, don't hesitate to submit a PR !

Anyway, imho this issue isn't really an issue of Reveal itself : I may be wrong but it never was intented to run server-side. Having a great tool like reveal-md using reveal.js is really cool but I wonder if this kind of issue shouldn't be adressed in their repo since it's using Reveal in a way that is not the "usual" one.
I know it isn't the kind of thing we like as devs, but adding a global.navigator to mock the navigator API could maybe help in this case. I never used it but saw https://www.npmjs.com/package/node-navigator that seem to do this kind of thing.

@t-fritsch
Copy link
Collaborator

@hakimel could you tell if this issue should be reopened or not ?

@Martinomagnifico
Copy link
Collaborator

@t-fritsch Maybe the plugin could be changed a little bit to only use the user options if getConfig is defined?

@hakimel
Copy link
Owner

hakimel commented Nov 6, 2023

I've added a nil check so the getSlidifyOptions method won't fail if deck.getConfig() is undefined: 9d1c7e2

@MartenBE
Copy link
Author

MartenBE commented Nov 6, 2023

I think deck itself is undefined. If I call the plugin in the original way (without import Reveal itself) and just update the reveal.js version from 4.5.0 to 4.6.1 I get the following:

import Markdown from 'reveal.js/plugin/markdown/markdown.js';

export const md = (() => {
  return Markdown();
})();
TypeError [Error]: Cannot read properties of undefined (reading 'getConfig')
      at r (/home/martijn/git/reveal-md/node_modules/reveal.js/plugin/markdown/markdown.js:7:771)
      at Object.i [as slidify] (/home/martijn/git/reveal-md/node_modules/reveal.js/plugin/markdown/markdown.js:7:1325)
      at slidify (file:///home/martijn/git/reveal-md/lib/render.js:33:13)
      at TestContext.<anonymous> (file:///home/martijn/git/reveal-md/test/slidify.spec.js:48:18)
      at Test.runInAsyncScope (node:async_hooks:206:9)
      at Test.run (node:internal/test_runner/test:631:25)
      at Test.processPendingSubtests (node:internal/test_runner/test:374:18)
      at Test.postRun (node:internal/test_runner/test:715:19)
      at Test.run (node:internal/test_runner/test:673:12)
      at async Test.processPendingSubtests (node:internal/test_runner/test:374:7)

hakimel added a commit that referenced this issue Nov 6, 2023
@hakimel
Copy link
Owner

hakimel commented Nov 6, 2023

@MartenBE Got it—I've added a check for that too and confirmed that this works:

const md = RevealMarkdown();
md.slidify('# Slide 1\n---\n#Slide 2')

@MartenBE
Copy link
Author

MartenBE commented Nov 6, 2023

Thank you very much! We'll upgrade our dependency at your next release. We'll also investigate if we can let go of the direct call in the future.

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

No branches or pull requests

4 participants