gohyperr / hecs

A high performance lightweight ECS with a plugin ecosystem!
MIT License
23 stars 1 forks source link

Source maps would help debug some classes of problems #13

Open canadaduane opened 4 years ago

canadaduane commented 4 years ago

Occasionally I do something dumb (ok, quite often) and end up in some code path that HECS didn't anticipate, throwing an error that is hard to debug.

For example:

image image

Adding source maps to the dist/ folder would help here (maybe).

ashconnell commented 4 years ago

Yeah I haven't gotten around to it yet. Personally I don't run into this much because i've been using it for a long time and must know how to avoid it, but i understand people just picking HECS up are likely to stumble into this quite often.

I might be able to look at adding source maps in the next week or two, it's probably not too hard.

Do you have a snippet that reproduces an error like the one above?

canadaduane commented 4 years ago

Do you have a snippet that reproduces an error like the one above?

Sure, here's my latest dumb mistake :)

Inside a System's update method:

const transform = entity.get(Transform);
transform.position.add(undefined)
ashconnell commented 3 years ago

These are the steps I took to create a reproduction:

  1. Edit the hecs-plugin-core package WorldTransformSystem and insert a throw new Error('LOL') into the update() function
  2. Run yarn build in hecs-plugin-core
  3. Run yarn start in hecs-example

This is what I see in the console:

image

(we're expecting to see minified code which is useless to us)

I then:

  1. Changed hecs-plugin-core webpack config to remove config.devtool = false
  2. Ran yarn build in hecs-plugin-core
  3. Refreshed the browser

This is what I see

image

All is good!

I also don't think we need to add the files change in this PR. By default, Npm deploys all files. If you run npm pack you can see what gets published and both src and dist are included already.

Just for brevity, I also double checked the source-maps are a separate file so as not to increase the bundle size.

ashconnell commented 3 years ago

@canadaduane if this works for you, lets remove the files change and then I can merge

canadaduane commented 3 years ago

Ok, I can confirm this works in hecs-example. I'm unable to see sources in my own project, but I suspect this may be my own misconfiguration.