Repository navigation
TypeScript: Zone#create should have required parameter of type CreateZoneRequest #126
Description
Activity
I'm working on this now. Feel free to assign to me if that's useful.
Actually it's not obvious to me how to proceed here.
Zone'screatemethod is inherited fromServiceObject, which knows nothing aboutdnsNames. I tried to add some appropriatecreateoverloads to theZoneclass definition, but encounteredProperty 'create' in type 'Zone' is not assignable to the same property in base type 'ServiceObject'.The way to call our
child.create()is with the arguments theparent.createChild()method expects, except the first argument, which is always the name.const zone = dns.zone('...') zone.create(config, callback) // === dns.createZone('...', config, callback)
The
createbehavior is defined inServiceObject.prototype.create, which callsconfig.createMethod:Line 345 in d076731
A condensed version of all of this functionality:
class ServiceObject { constructor(cfg) { this.id = cfg.id this.createMethod = cfg.createMethod } create() { const args = [this.id] // figure out any additional arguments to provide to // the create method, i.e. args.push(options) this.createMethod.apply(null, args) } } class DNS extends Service { createZone(name, config, callback) { config.name = name this.request(...) } } class Zone extends ServiceObject { constructor(parentInstance) { super({ methods: { create: true }, createMethod: parentInstance.createZone.bind(parentInstance) }) } }
@stephenplusplus TIL about embedded code snippits. NEAT! The JavaScripty part of all this is working just fine. It's the types that aren't correct. Here is a simplification that illustrates the issue:
class Service {} interface CreateZoneOptions { dnsName: string; } class DNS extends Service { createZone(options: CreateZoneOptions) { if (typeof options === 'undefined') { // This code should never be reached by a TypeScript user. // Said another way, the type of "options" here is "never". // But let's be kind to our JavaScript users and fail fast. throw new Error('Expected argument "options"'); } const { dnsName } = options; if (typeof dnsName === 'undefined') { // This code should never be reached by a TypeScript user. // Said another way, the type of "dnsName" here is "never". // But let's be kind to our JavaScript users and fail fast. throw new Error('Expected argument "options" to include "dnsName"'); } // Do request to /managedZones and so forth ... } } interface ServiceObjectOptions { createMethod: Function; } class ServiceObject { createMethod: Function; constructor(opts: ServiceObjectOptions) { this.createMethod = opts.createMethod; } create(opts?: any) { this.createMethod.apply(null, opts); } } class Zone extends ServiceObject { constructor(dns: DNS) { super({ createMethod: dns.createZone.bind(dns) }); } } const dns = new DNS(); const zone = new Zone(dns); // For "dns", everything works as expected. // Compiler error "Expected 1 arguments, but got 0." dns.createZone(); // Compiler error: "'dnsLameTypo' does not exist in type 'CreateZoneOptions'." dns.createZone({ dnsLameTypo: 'carnesen.com' }); // OK! dns.createZone({ dnsName: 'carnesen.com' }); // For "zone", the typings are not sufficiently strict. // The following is NOT currently a compiler error. It should be. // Instead we encounter a runtime error 'Expected argument "options"' zone.create(); // The following is NOT currently a compiler error. It should be. // Instead we encounter a runtime error 'Expected argument "options" to include "dnsName"' zone.create({ dnsLameTypo: 'carnesen.com' }); // OK! zone.create({ dnsName: 'carnesen.com'});
To continue that example, here is one way we could approach it using TypeScript generics:
interface CreateMethod<T> { (opts: T): void; } interface ServiceObjectOptions<T> { createMethod: CreateMethod<T>; } class ServiceObject<T> { private createMethod: CreateMethod<T>; constructor(opts: ServiceObjectOptions<T>) { this.createMethod = opts.createMethod; } create(opts: T) { this.createMethod(opts); } } class Zone extends ServiceObject<CreateZoneOptions> { constructor(dns: DNS) { super({ createMethod: dns.createZone.bind(dns) }); } } const zone = new Zone(dns); // Compiler error "Expected 1 arguments, but got 0." zone.create(); // Compiler error "Property 'dnsName' is missing in type '{}'." zone.create({}); // Compiler error "'dnsLameTypo' does not exist in type 'CreateZoneOptions'" zone.create({ dnsLameTypo: 'carnesen.com' }); // OK! zone.create({ dnsName: 'carnesen.com'});
@carnesen I know it's been quite some time since you filed this bug, but it should now be addressed in
@google-cloud/[email protected], mind taking this for a spin and letting us know if it addresses your issues?
Currently the TypeScript type of
Zone#createis inherited fromServiceObject#create. Effectively the first argument is optional and if present can have any shape. (TypeScript doesn't do the excess property check on assignments to an empty object type.) That doesn't jibe with either the docs or the runtime checks. Instead, the first parameter ofZone#createshould be required of typeCreateZoneRequest. (Maybe without the "name" property?).