Skip to content

make db interface async#86

Merged
nicolodavis merged 10 commits into
boardgameio:masterfrom
saeidalidadi:saeidalidadi/persist-state
Feb 8, 2018
Merged

make db interface async#86
nicolodavis merged 10 commits into
boardgameio:masterfrom
saeidalidadi:saeidalidadi/persist-state

Conversation

@saeidalidadi
Copy link
Copy Markdown
Contributor

No description provided.

@nicolodavis nicolodavis changed the title persist game state inside db make db interface async Jan 19, 2018
@nicolodavis
Copy link
Copy Markdown
Member

Sorry, I haven't had time to look more closely at this. Are you stuck trying to fix the tests?

Comment thread src/server/index.test.js
});

test('action', () => {
test('action', async () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async not needed, because not await exist

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but I forgot to remove it after I clean await s when it didn't work

Comment thread src/server/index.test.js
// Sync causes the server to respond.
expect(io.socket.emit).toHaveBeenCalledTimes(0);
io.socket.receive('sync', 'gameID');
expect(io.socket.emit).toHaveBeenCalledTimes(1);
Copy link
Copy Markdown
Contributor Author

@saeidalidadi saeidalidadi Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nicolodavis sorry for my delayed reply
Wrapping this assertion inside a timeout works fine but I think there should be a better solution for async behavior of socket callbacks

Copy link
Copy Markdown
Member

@nicolodavis nicolodavis Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use await in the tests. The trick is to make the function explicitly have a return statement. Then you can use await to wait for its execution and then test what it did. I wrapped receive in this way to make the tests to work.

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 4, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 65d0e6a on saeidalidadi:saeidalidadi/persist-state into 9e507ce on google:master.

Comment thread package.json Outdated
"babel-plugin-external-helpers": "^6.22.0",
"babel-plugin-module-resolver": "^3.0.0",
"babel-plugin-transform-es2015-modules-commonjs": "^6.26.0",
"babel-polyfill": "^6.26.0",
Copy link
Copy Markdown
Member

@nicolodavis nicolodavis Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to do this without babel-polyfill? Recent versions of Node support async / await out of the box.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After running examples I got regeneratorRuntime error.
I tried to solve it by testing all options from babel docs and finally this one worked.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

babel-polyfill shouldn't be used in a library, so we should find another way to do this. Have you tried babel-preset-env?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other option would be to not use Babel on anything in src/server and write it without ES5 features. I can take a look at this later.

@nicolodavis
Copy link
Copy Markdown
Member

nicolodavis commented Feb 7, 2018

I think this is working correctly now!

@nicolodavis nicolodavis merged commit c2ea197 into boardgameio:master Feb 8, 2018
sedobrengocce pushed a commit to sedobrengocce/boardgame.io that referenced this pull request Mar 21, 2018
* persist game state inside db

* undo some unnecessary changes

* fix merge

* fix tests

* no babel on src/server

* reset package-lock.json

* remove stage-0 preset
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 this pull request may close these issues.

5 participants