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

DOMParser doesn't exist in headless/non-browser environments #483

Closed
jfelsinger opened this issue Jan 7, 2024 · 5 comments · Fixed by #487
Closed

DOMParser doesn't exist in headless/non-browser environments #483

jfelsinger opened this issue Jan 7, 2024 · 5 comments · Fixed by #487

Comments

@jfelsinger
Copy link
Contributor

First, thanks for the good work, most recent release is really awesome.

I just pulled the latest version to check out the new features, and headless mode, but map loading on the server-side (node) looks to be broken because the parser is using DOMParser which unfortunately isn't available outside the browser.

Environment

  • browsers and versions: Node JS v20
  • operating system:
  • Tiled plugin version: 0.29.0-alpha.1
  • Excalibur versions: Latest

Current Workaround

None yet, I'm looking at some light-weight server-side DOM parser implementations like Happy-Dom, Cheerio, or JSDOM; and a way to use them within the parsing pipeline just within a node env.

@eonarheim
Copy link
Member

@jfelsinger Ope! Thanks for the issue! I should have checked DOMParser worked in node 🤦!

I'll look into this today to see if there's an easy polyfill I can sub in.

@eonarheim
Copy link
Member

eonarheim commented Jan 7, 2024

On the surface JSDOM looks like it might be the most drop-in...

Also xmldom, which is a bit smaller https://npmjs.com/package/xmldom

@jfelsinger
Copy link
Contributor Author

I was looking at xmldom, but it looks like it only supports up to the DOM level 2 spec, which doesn't have querySelector or querySelectorAll.

@jfelsinger
Copy link
Contributor Author

jfelsinger commented Jan 10, 2024

I implemented this on a forked branch with JSDOM as an optional dep if you want to check it out. It shouldn't add JSDOM to the bundle, or change the bundle for the browser, but if on node, it'll use JSDOM if it's available, so any node consumer should just have to do npm i jsdom and it'll work.

I had to change a couple other things other than including the parser, because I noticed that, despite the headless option, images were still trying to be loaded, causing some errors.

This is working at least for my project, both in browser and node, but might need a little cleaning-up. (If you want I can clean it up and make a proper PR)

https://github.com/excaliburjs/excalibur-tiled/compare/main...jfelsinger:excalibur-tiled:node-parsing?expand=1

@eonarheim
Copy link
Member

@jfelsinger wow! Thanks for digging in to the headless work! JSDOM definitely seems like the right touch, we can add documentation to that effect.

I'd love a PR, let's do it!

jfelsinger pushed a commit to jfelsinger/excalibur-tiled that referenced this issue Jan 16, 2024
jfelsinger pushed a commit to jfelsinger/excalibur-tiled that referenced this issue Jan 16, 2024
jfelsinger pushed a commit to jfelsinger/excalibur-tiled that referenced this issue Jan 16, 2024
jfelsinger pushed a commit to jfelsinger/excalibur-tiled that referenced this issue Jan 16, 2024
eonarheim added a commit that referenced this issue Jan 17, 2024
closes #483 

This PR adds [jsdom](https://www.npmjs.com/package/jsdom) as an optional dependency that can be detected and used in environments that don't have support for the browser-internal `DOMParser`.

Caveats: This also fixes the graphics component breaks I was running into when trying to run headless to actually test this. I can move that out into a separate Issue & PR if that makes sense to do. Essentially, I removed the loading of the actual image/sprite/animations when headless since it was breaking in Node when a browser graphic context wasn't there, even though headless would imply that not being necessary.

## Usage

When planning to use excalibur-tiled in Node.js or another non-browser environment, include jsdom when installing excalibur-tiled...

`npm i --save @excaliburjs/plugin-tiled@next jsdom`

or any time after if it's needed.

`npm i --save jsdom`

---------

Co-authored-by: Joel Felsinger <jfelsinger@spigot.com>
Co-authored-by: Erik Onarheim <erik.onarheim@gmail.com>
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

Successfully merging a pull request may close this issue.

2 participants