excaliburjs / Excalibur

🎮 Your friendly TypeScript 2D game engine for the web 🗡️
https://excaliburjs.com
BSD 2-Clause "Simplified" License
1.83k stars 192 forks source link

Investigate replacing/changing `instanceof` checks #2436

Closed eonarheim closed 9 months ago

eonarheim commented 2 years ago

Steps to Reproduce

Using instanceof internally in Excalibur has been a source of bugs depending on the bundle/minifier. Vite and Angular appear to need additional configuration to work around their default minification settings so they preserve constructor names.

See

Expected Result

Excalibur should be resilient to build tool minification steps. At minimum there should be a warning logged with documentation linked when an instanceof check fails.

Actual Result

Excalibur silently fails and users are mystified to what is wrong.

Environment

Current Workaround

Update used bundler configuration to preserve names

mattjennings commented 2 years ago

Perhaps this useful insight: I ran into instanceof not working properly when npm linking @excaliburjs/plugin-tiled to my project. This caused the tilemap to not render anything at all, exactly like #344. The problem ended up being multiple excalibur installs in node_modules, rather than anything with minification. This is a common problem with linking, as you might be linking to a project with its devDependencies installed (as was the case for me with @excaliburjs/plugin-tiled), but it could also happen if there are different versions of excalibur installed too.

Since I was using Vite, the solution was to configure a dedupe, ensuring only 1 instance was bundled in my project.

React does some magic to detect possible multiple installs of itself, perhaps there's something to learn from there to do the same? This is the error it throws, though I'm not sure how it gets to throwing it.

github-actions[bot] commented 2 years ago

This issue hasn't had any recent activity lately and is being marked as stale automatically.

eonarheim commented 9 months ago

I think we got to the bottom of this, instanceof is okay as long as you have the right peerDependency setup and vite has dedup enabled