[WIP] initial typescript conversion

[rgbkrk]

Following on from #58, I've kicked off the conversion of diff-match-patch starting with @NeilFraser's starting point as the first commit. I've included the built copy for now, though I may strip that out and rebase it based on feedback.

One interesting thing between Neil's built version and my built version:

diff --git a/typescript/built/diff_match_patch.js b/typescript/built/diff_match_patch.js index 8fbb030..dafa48a 100644 --- a/typescript/built/diff_match_patch.js +++ b/typescript/built/diff_match_patch.js @@ -387,7 +387,7 @@ var diff_match_patch = /** @class */ (function () { */ diff_match_patch.prototype.diff_linesToChars_ = function (text1, text2) { var lineArray = []; // e.g. lineArray[4] == 'Hello\n' - var lineHash = {}; // e.g. lineHash['Hello\n'] == 4 + var lineHash = Object.create(null); // e.g. lineHash['Hello\n'] == 4 // '\x00' is a valid character, but various debuggers don't like it. // So we'll insert a junk entry to avoid generating a null character. lineArray[0] = '';

Not quite sure why typescript goes that route to make an object with a null prototype (that still behaves like an object).

What works

• Base tests ✅
• Speed tests ❌
  • The speed tests use diff_match_patch_uncompressed.js which is not generated (by name) here
• Typing! ✅

Next steps

• Determine how well the types line up with that of @types/diff-match-patch to ease migration
• Make speed tests run
• Determine how the uncompressed and compressed versions will be stored (I need direction here)

[captainsafia]

From my understanding, using “{}” to create a dictionary means if inherits the properties of the Object prototype, whereas using Object.create with a null parameter does not inherit any of the properties.

I looked up some more stuff about it and learned that maps created this way have faster access times in some cases. Interesting.

https://stackoverflow.com/questions/34480709/why-is-object-create-so-much-slower-than-a-constructor

By the way, cool work!

[NeilFraser]

{} vs Object.create(null) is an interesting one. Performance considerations aside, Object.create(null) is prefered when using an object as a map with arbitrary keys, since one of those keys might be 'toString' or one of the other six properties inherited from Object.prototype. However, DMP was written before Object.create(null) existed. Thus it uses the more dangerous {}.

There are two places where {} is used as a map. One place is in match_alphabet_. This one is harmless since the keys are always single characters. At this time, there are no properties on Object.prototype with single character names. The other place is diff_linesToChars_. This one is dangerous. As such there's a hasOwnProperty check before each access so that we don't read in inherited properties. However, hasOwnProperty was only added in ES3, so Netscape 4 and early Safari versions don't support it. That's why we test for the existence of the hasOwnProperty function before calling it.

These days of course this level of backwards compatibility is completely ludicrous.

[rgbkrk]

I just went on paternity leave for the last few months, I'm jumping back into this now. 😄

[rgbkrk]

Looked at the git diff since I last touched this, seems I can safely rebase and go forward as the changes were to a header file and the dart code.

[samelhusseini]

@rgbkrk Typescript doesn't do anything special here.
You TS file just has Object.create(null) instead of {} hence why the two JS built files are different:
see https://github.com/google/diff-match-patch/pull/60/files#diff-1a73c059bb6730ae237f4a4e3497e1aaR449

[samelhusseini]

Ah I think I understand the source of confusion here. The JS built file in Neil's zip is outdated. Tr building it and you should get the same result.

[samelhusseini]

Also I worked on a conversion not realizing this PR existed, if you'd like to take a look, I'd love to hear your thoughts on: #74

One design decision I want to point out is making Diff a class with text and operation properties. On the one hand that's more consistent with other language conversions and is cleaner to define and access than using a Tuple (Array), but I'm wondering if that's worth it if it makes migration more tedious.

[rgbkrk]

@samelhusseini hey no problem, I've ended up being swamped with other work so I'm glad you've picked this up.

[rgbkrk]

Closing in favor of #74