Skip to content

Commit

Permalink
Switch src/ to use ES classes (googleapis#158)
Browse files Browse the repository at this point in the history
Fixes googleapis#85 

- [x] Tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)

---

This PR should be ready to go. I believe this requires a major semver bump. I figured the test files shouldn't be updated in the same PR. The only thing that really needs to be pointed out is that classes shouldn't be instantiatable without the new keyword:

```js
const client1 = new Bigtable()
const client2 = Bigtable() // Error: Class constructor Foo cannot be invoked without 'new'
```

I was on the fence about requiring people to use the `new` keyword or not, and chose to make it optional. I did this by using a proxy:

```js
Bigtable = new Proxy(Bigtable, {
  apply(target, thisArg, argumentsList) {
    return new target(...argumentsList);
  },
});
```

Let me know if this should be removed. I didn't add that to the semi-private classes since the common case of creating one is something like `instance.table(...)` and not `new Table(...)`

Note - This was removed in review

As before, most of this was done via lebab so there may be some issues so please do go through it again to make sure nothing terrible is being done.
  • Loading branch information
kolodny authored Jun 11, 2018
1 parent 20fd2e4 commit 622cd39
Show file tree
Hide file tree
Showing 12 changed files with 5,666 additions and 5,650 deletions.
3 changes: 2 additions & 1 deletion samples/system-test/instances.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ const test = require(`ava`);
const tools = require(`@google-cloud/nodejs-repo-tools`);
const uuid = require(`uuid`);

const bigtable = require(`@google-cloud/bigtable`)();
const Bigtable = require(`@google-cloud/bigtable`);
const bigtable = new Bigtable();
const clusterName = `nodejs-bigtable-samples-${uuid.v4()}`.substr(0, 30); // Bigtable naming rules
const instanceName = `nodejs-bigtable-samples-${uuid.v4()}`.substr(0, 30); // Bigtable naming rules
const instance = bigtable.instance(instanceName);
Expand Down
Loading

0 comments on commit 622cd39

Please sign in to comment.