googlemaps/v3-utility-library

MarkerWithLabel to TypeScript

jpoehnelt opened this issue · 9 comments

jpoehnelt commented
MarkerWithLabel to TypeScript
bamnet commented

Does 'to TypeScript' mean convert the JavaScript library to TypeScript, just write Typedefs, or something else entirely?

(debating volunteering here, but want to make sure I know what I'm biting off)

jpoehnelt commented

I would prefer TypeScript over just typedefs. Unfortunately, it is not a straightforward task. While @types/googlemaps is available, there are issues around es6 and typescript.

Implement: Foo implements google.maps.*

We cannot copy the prototype properties over during instantiation with TypeScript and still use implements. We do this copy in the library now.

Extend: Foo extends google.maps.*

Extends does not work if parent is not available.

Hack around extends(mostly functional)

declare class Foo extends google.maps.OverlayView { }

class Bar { 
    method() {
        console.log("Bar.method")
    }
}

function Foo(): Foo {
  class Foo_ extends Bar { //google.maps.OverlayView {
    constructor() {
      super()
      Object.defineProperty(this.constructor, 'name', { value: 'Foo' });
    }
    static getClassName() { return 'Foo'; }
    getClassName() {
      return Foo_.getClassName();
    }
  }
  return new Foo_() as Foo;
}

https://stackoverflow.com/questions/29310530/get-the-class-name-of-es6-class-instance

There might be other ugly solutions to this.

bamnet commented

I've think I've wrapped my head around this problem. Here's where I'm at:

Currently, MarkerWithLabel requires google.maps.Marker and google.maps.OverlayView to be defined before it can be loaded. If you load the library before those two google.maps dependencies exist it still loads correctly but isn't usable because inherits() hasn't run.

To workaround this, developers use a variety of different tricks to ensure the scripts are loaded in-order, including the simple ordered synchronous <script> tags (classic approach) or delayed require() tags that wait until the bootstrap has loaded.

In an Ideal World, developers wouldn't have to think about this. MarkerWithLabel could be included in a compiled codebase at anypoint, loaded in any order after it's require() statement, just like any other modern JS or TS library.... probably at the top of your code base.

In modernizing this library, we can take a few steps:

a. Write Typedefs for our current codebase.
b. Convert the current implementation over to TypeScript. extends should provide similar functionality to the inherits() method the current code uses.
c. Build a better TypeScript version that had the loading behavior developers expect, using hacks like shown above.

To evaluate tackling (b) or (c), I'm going to whip up a few demos to see what I can do for (c) building a more modern version of the USGSOverlay demo.

jpoehnelt commented

In an Ideal World, developers wouldn't have to think about this. MarkerWithLabel could be included in a compiled codebase at anypoint, loaded in any order after it's require() statement, just like any other modern JS or TS library.... probably at the top of your code base.

this exactly

d. give feedback for long term model change in how the map script is bootstrapped
e. shim the entire google maps externs interface to only fail at runtime if you call the constructor or method

I'm still exploring e to see if it is possible and as input for d

To evaluate tackling (b) or (c), I'm going to whip up a few demos to see what I can do for (c) building a more modern version of the USGSOverlay demo,

please share. we can take this off github if necessary

timofei-iatsenko commented

Approach using delcaration merging:

interface OverlayViewSafe extends google.maps.OverlayView {}

class OverlayViewSafe {
  constructor() {
    // MarkerClusterer implements google.maps.OverlayView interface. We use the
    // extend function to extend MarkerClusterer with google.maps.OverlayView
    // because it might not always be available when the code is defined so we
    // look for it at the last possible moment. If it doesn't exist now then
    // there is no point going ahead :)
    this.extend(OverlayViewSafe, google.maps.OverlayView);
  }

  private extend(obj1, obj2) {  /*....*/ }
}

export class MarkerClusterer extends OverlayViewSafe { /*....*/ }

That works without too ugly hacks. It will not throw an error until you will try instantiate it.

stale commented

This issue has been automatically marked as stale because it has not had recent activity. Please comment here if it is still valid so that we can reprioritize. Thank you!

stale commented

Closing this. Please reopen if you believe it should be addressed. Thank you for your contribution.

jpoehnelt commented

Currently rewriting this package in TypeScript.