Skip to content

Fixing issue with getSync in runtime #14

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

Merged
merged 5 commits into from
Jul 16, 2019
Merged

Conversation

ShelbyZ
Copy link
Contributor

@ShelbyZ ShelbyZ commented May 10, 2019

  • needed default serviceName not object destructuring

ShelbyZ and others added 4 commits October 3, 2018 13:49
@dkundel
Copy link
Contributor

dkundel commented Jun 28, 2019

Hi @ShelbyZ sorry for the delay here but I think at least according to the docs you are supposed to pass in getSync({ serviceName: 'something' }) and therefore it should be destructured here.

https://www.twilio.com/docs/runtime/client?code-sample=code-get-an-existing-sync-document-12&code-language=Node.js&code-sdk-version=default

@ShelbyZ
Copy link
Contributor Author

ShelbyZ commented Jun 28, 2019

It looks like that line of code is trying to provide a default value for serviceName. The call does not set the value of serviceName to 'default' so later return client.sync.services(serviceName); will have undefined for the value of serviceName.

@dkundel
Copy link
Contributor

dkundel commented Jun 28, 2019

Oh yeah my bad. So the code should be

  function getSync(syncService) {
    syncInfo = syncInfo || { serviceName: 'default' };
    const client = twilio(env.ACCOUNT_SID, env.AUTH_TOKEN);
    return client.sync.services(serviceName);
  }

Do you want to fix your PR or should I push this change in?

@ShelbyZ
Copy link
Contributor Author

ShelbyZ commented Jun 28, 2019

function getSync(syncService) {
    syncInfo = syncService || { serviceName: 'default' };
    const client = twilio(env.ACCOUNT_SID, env.AUTH_TOKEN);
    return client.sync.services(syncInfo.serviceName);
  }

The call to client.sync.services(syncInfo.serviceName); - is either 'default' or the sync service sid

@dkundel
Copy link
Contributor

dkundel commented Jul 1, 2019

Yeah that should be the behavior

@dkundel dkundel merged commit bd2e04d into twilio-labs:master Jul 16, 2019
@dkundel
Copy link
Contributor

dkundel commented Jul 16, 2019

Thank you so much for the fix! I'll publish a new version of 1.x later 😊

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.

3 participants