rapideditor / rapid-sdk

🛠️ Map editing made easy
ISC License
33 stars 8 forks source link

Create osm entity model #106

Closed mbrzakovic closed 5 months ago

mbrzakovic commented 2 years ago

Complete osm entity module development. Transfer appropriate files to .ts

bhousel commented 2 years ago

Nice, yeah I'm working on this now..

The main struggle is upgrading the old ES5 module-with-stuff-in-prototype to modern ES6 classes.

The old code has a lot of "magic" like factories that let you instantiate without new, and an initializer that copies the existing Object's properties into the new Object's ownProperties. These practices feel not very compatible with TypeScript, and I don't know how much of that I want to preserve.

Screen Shot 2021-11-18 at 11 58 06 AM Screen Shot 2021-11-18 at 12 00 41 PM

Still, it's important for us to keep this library mostly compatible with our existing code so that we can 1. drop it into RapiD and start using it, and 2. to make converting the next pieces (service, graph, history, actions) reasonable.

mbrzakovic commented 2 years ago

Yes, I understand what you are saying. Please feel free to brain-dump some of these dilemmas if you feel they are blocking you, I will try to provide comments promptly.

In the abovementioned case, I don't have a strong opinion, though I think its better to bite the bullet now and try to get rid of prototypical inheritance and "magic" factories on sdk side. I just did a quick search on where osmEntity/Node/Way/Relation are being used and it seems that the pieces of code that are instantiating osm entities are: services/osm and actions (with few minor exceptions). Changing the core code shouldn't represent much effort, however these are intensively being used in unit tests and making this syntax modification can be cumbersome.

Perhaps some temporary 'bridge' code could be used on consumer side in order to avoid these syntax modifications until the actions and osm service become available via sdk: e.g. (simplified):

export function osmNode() {
  return new Node(arguments);
}
bhousel commented 5 months ago

Closing as stale, but we still do plan to do this.

In the past few years in Rapid, we've rewritten most of the core systems around JavaScript classes (following PixiJS's lead, which is also a TypeScript project) and it's been pretty great.