Plugins: Maybe we shouldn't use the global `document`?
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 13k
- Forks
- 1.4k
- Avg merge
- 15h 36m
- Merged PRs (30d)
- 3
Description
Motivation
I was having a conversation with someone that wanted to use Keep Markup on the server side. Since KM depends on the DOM, I suggested using JSDOM, and that worked fine. But they had to include all of Prism in JSDOM. It basically worked like this:
const { JSDOM } = require("jsdom");
const dom = new JSDOM(`
<pre><code class="language-xxxx">some code <a href="">with links></a>!</code></pre>
<script>${PrismCoreSourceCode}</script>
<script>${PrismKeepMarkupSourceCode}</script>
`, { runScripts: "dangerously" });
This is really complex because you have to have Prism's source code. You can't just require Prism like any other dependency.
The reason why you have to do that is this line in KM.
var range = document.createRange();
document refers to a value in the global scope called document (= the document property of the global object). So you can't do require("prismjs/plugins/keep-markup/prism-keep-markup.js") because then the global object will be NodeJS' global and global doesn't have a document property (by default).
However, it would be a lot nicer if we could just do this:
const { JSDOM } = require("jsdom");
const Prism = require("prismjs");
require("prismjs/plugins/keep-markup/prism-keep-markup.js");
const dom = new JSDOM(`<code class="language-xxxx">some code <a href="">with links></a>!</code>`);
const code = dom.window.document.querySelector("code");
Prism.highlightElement(code);
const highlightedHTML = code.innerHTML;
This is only the motivating example. There are a few other plugins that don't easily work with JSDOM for the same reason.
Description
Maybe we should replace references to the global document object with element.ownerDocument?
The basic idea is that we don't need document unless we are working with DOM nodes in which case element.ownerDocument is accessible to us. This is really easy to implement without changing much code.
Example:
This is the full "fix" for KM.
Prism.hooks.add('after-highlight', function (env) {
+ var document = env.element.ownerDocument;
+ // document will now refer to this variable without changing any other code
This should only be done with plugins that can function in a virtual DOM. It doesn't make sense to implement this for plugins that rely on user events.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with plugins/keep-markup/prism-keep-markup.js at the createRange call, then inspect other plugins for the same global document usage. Done means virtual-DOM-compatible plugins work through the shown require/JSDOM flow without a global document, while plugins that rely on user events remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- frontend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100