search

ssbc / scuttle-poll

3 stars 1 forks source link

WIP start adding action to poll object #18

Closed mixmix closed 6 years ago

mixmix commented 6 years ago

Hey @pietgeursen this was kicking my arse all afternoon. There's something very strange going on.

pietgeursen commented 6 years ago

Oh yep. Think I'm onto it. It's a circular dependency.

poll.async.get requires position.async.chooseOne which requires position.async.position AND position.async.position requires poll.async.get

If you log out what GetPoll is from position.async.position by running: $ node ./position/async/position it is the function we expect.

But if you run $ node ./poll/async/get it is an empty object.

pietgeursen commented 6 years ago

My gut says this about the inject function not working well when there are circular deps.

I have a hunch that depject would do this correctly but I'm not sure it's worth the extra boilerplate.

Another option is to just use dependency injection ourselves and have some common parent passing the required deps down. But then we might as well just use depject.

Or given that https://github.com/ssbc/scuttle-poll/blob/43720fd9fb8b5f2c7132fb7d4fc53b1b07a550b9/position/async/position.js#L14 works and now we understand why, just leave it like that.

pietgeursen commented 6 years ago

Also heads up: scuttle-testserver is updated now so you can remove ./lib/testServer

mixmix commented 6 years ago

oh yeah! I'm middle of a refactor of build and publish methods. It's breaking fucking all the tests D: But it's also showing me where code breaks when people feed stupid things into the methods. Strong typing / asserting looks more preferable all the time.

position is not async and involves hitting server in order to construct the branch for you. Makes all the tests slightly more tricky around it