marijnh / orderedmap Goto Github PK

Currently this package exports a class as the module, which isn't supported in ES modules. A simple solution is to just use a named export, i.e.

exports.OrderedMap = OrderedMap;

Commonjs consumers would then import via:

const { OrderedMap } = require('orderedmap');

The motivation for this is to smooth the path for packages wanting to adopt ES modules that depend on this package.

It can be frustrating to loop over each key, value when searching for one value in particular.

It seems like this latest commit has broke prosemirror and

subsequently telerik kendo editor and our build.

The error:

The types introduced in #6 include a getter that is not supported in TypeScript versions 3.6 and earlier. This particular change in the compiler output was widely criticized at the time.

While these TypeScript versions are pretty old (~2 years) by now, they're still used by some projects. Our particular issue is with the Kendo UI Editor for Angular when running in Angular 8.x projects, but it may not be limited to it.

My suggestion is to replace it with a readonly field to remain compatible with older versions of TypeScript.

The only downside to that is that inheriting OrderedMap will require using a field for overriding size. I'm not sure if inheriting the class is a common scenario for the library. Edit: I've just noticed that the constructor is marked as private and extending the class is not supported.

diff --git a/types/index.d.ts b/types/index.d.ts
index 551e742..fdc372c 100644
--- a/types/index.d.ts
+++ b/types/index.d.ts
@@ -21,7 +21,7 @@ declare class OrderedMap<T = any> {
 
   subtract(map: MapLike<T>): OrderedMap<T>
 
-  get size(): number
+  readonly size: number
 
   static from<T>(map: MapLike<T>): OrderedMap<T>
 }

5c1541c

Should use export = for module.exports =.

This commit also breaks @types/prosemirror-view.

WIth modern linters, especially with TypeScript code, (e.g. the unicorn/recommended rules), I've noticed it trying to replace my .forEach loops for a for (const item of orderedMapObject) which breaks my code.
Any reason to not implement support for the Symbol.iterator method so that using for() loops will work smoothly with this library?
If this is accepted, I don't PR-ing a fix for this.

The from function should use the value of a 'toObject' function if it exists on the given object. This will add compatibility with other mapped classes, and more importantly, make orderedmap compatible with other versions of itself (that also have the toObject function) so that it does not need to be a shared dependency (for consumers using the newer version).

example implementation (checks that a 0 argument toObject function exists and then calls it)
if (value && typeof value.toObject === 'function' && value.toObject.length === 0) value = value.toObject()