[Bug] Cannot import two.js in typescript project

[nmay231]

Describe the bug
I guess this is a continuation of #526. The generated types in two.js@0.7.5 don't work right off the bat. At least not with the default create-react-app typescript setup.

To Reproduce
Steps to reproduce the behavior:

1. npx create-react-app twojs-demo --template typescript
2. cd twojs-demo
3. npm i two.js@0.7.5
4. Replace src/App.tsx with the following:

import { useEffect } from 'react';
import Two from 'two.js'

function App() {
  useEffect(() => {
    const two = new Two({
      fullscreen: true,
      autostart: true,
    })

    const container = document.getElementById('container');
    if (container) two.appendTo(container)

    var rect = two.makeRectangle((two as any).width / 2, (two as any).height / 2, 50 ,50);
      (two as any).bind('update', function() {
        rect.rotation += 0.001;
      });
  }, [])

  return (
    <div id='container' />
  );
}

export default App;

5. Observe the error: File 'twojs-demo/node_modules/two.js/types.d.ts' is not a module.

Potential fix
Modify node_modules/two.js/types.d.ts with the following changes:

- declare namespace Two {
+ declare module 'two.js' {
+   export default Two;

...

Reload the dev server or resave App.tsx to enact HMR. The code should run with no problems.

Notes

Besides not being able to import two.js, the type definitions were lacking the .width, .height, and .bind attributes of a new Two() instance. That's why the App.tsx casted (two as any). There might be other small problems like that.

I never used the jsdoc typescript generator, but perhaps there is a way to switch to declare module instead of declare namespace? I think 'module' is more modern in typescript (I would have to double-check).

Additionally, there should be a new version of @types/two.js to mention that that package is just a stub file since you are including types here. I can open a PR over there if you are ready to take that step.

[jonobr1]

Thanks for sharing this. As you can see, the type declarations are still very rough 😅 . This example project setup is really helpful for me though, thank you! I can now actually see the errors TypeScript users are getting.

To be totally up front, the way this codebase is structured (especially the way the jsdocs are being generated) doesn't mesh well with TypeScript. The compiler cannot tell, merge, or discern being a module and also being a class, which is why I think it defaults to namespace. In fact, browsing the TypeScript documentation on namespaces it looks like a namespace cannot also be a class. This is problematic because Two.js is constructed like:

var two = new Two();     // An instance of Two.js
var Anchor = Two.Anchor; // Also a namespace to access other classes

[aaron-harvey]

I very quickly took a stab at this (diff below) and seemed to be successful. I'm new to the library so I wasn't able to test everything.

My goal was to make as little changes as possible but I believe there is some room for improvement using module syntax. I may be able to find sometime this week to tinker around.

diff --git a/types.d.ts b/types.d.ts
index 505535c7f140036ec33dc43e34d6a0d45aabd059..3f5d4e7d70aeb4dee19cdeec27f584743c5fe3e3 100644
--- a/types.d.ts
+++ b/types.d.ts
@@ -1,3 +1,7 @@
+export as namespace Two;
+
+export = Two;
+
 declare namespace Two {
     /**
      * An object that holds 3 {@link Two.Vector}s, the anchor point and its corresponding handles: `left` and `right`. In order to properly describe the bezier curve about the point there is also a command property to describe what type of drawing should occur when Two.js renders the anchors.
@@ -107,8 +111,7 @@ declare namespace Two {
     /**
      * An `Array` like object with additional event propagation on actions. `pop`, `shift`, and `splice` trigger `removed` events. `push`, `unshift`, and `splice` with more than 2 arguments trigger 'inserted'. Finally, `sort` and `reverse` trigger `order` events.
      */
-    class Collection extends Two.Events {
-    }
+    class Collection extends Events {}
     /**
      * This is the base class for constructing different types of gradients with Two.js. The two common gradients are {@link Two.LinearGradient} and {@link Two.RadialGradient}.
      * @param [stops] - A list of {@link Two.Stop}s that contain the gradient fill pattern for the gradient.
@@ -518,41 +521,6 @@ declare namespace Two {
      */
     interface Register {
     }
-    /**
-     * Object inherited by many Two.js objects in order to facilitate custom events.
-     */
-    class Events {
-        /**
-         * Call to add a listener to a specific event name.
-         * @param [name] - The name of the event to bind a function to.
-         * @param [handler] - The function to be invoked when the event is dispatched.
-         */
-        on(name?: string, handler?: (...params: any[]) => any): void;
-        /**
-         * Call to remove listeners from a specific event. If only `name` is passed then all the handlers attached to that `name` will be removed. If no arguments are passed then all handlers for every event on the obejct are removed.
-         * @param [name] - The name of the event intended to be removed.
-         * @param [handler] - The handler intended to be reomved.
-         */
-        off(name?: string, handler?: (...params: any[]) => any): void;
-        /**
-         * Call to trigger a custom event. Any additional arguments passed after the name will be passed along to the attached handlers.
-         * @param name - The name of the event to dispatch.
-         * @param arguments - Anything can be passed after the name and those will be passed on to handlers attached to the event in the order they are passed.
-         */
-        trigger(name: string, arguments: any): void;
-        /**
-         * @property undefined - Object of different types of Two.js specific events.
-         */
-        static Types: {};
-        /**
-         * Alias for {@link Two.Events.on}.
-         */
-        static bind(): void;
-        /**
-         * Alias for {@link Two.Events.off}.
-         */
-        static unbind(): void;
-    }
     /**
      * This is the primary class for grouping objects that are then drawn in Two.js. In Illustrator this is a group, in After Effects it would be a Null Object. Whichever the case, the `Two.Group` contains a transformation matrix and commands to style its children, but it by itself doesn't render to the screen.
      * @param [children] - A list of objects that inherit {@link Two.Shape}. For instance, the array could be a {@link Two.Path}, {@link Two.Text}, and {@link Two.RoundedRectangle}.
@@ -1097,7 +1065,7 @@ declare namespace Two {
      * @param [parameters] - This object is inherited when constructing a new instance of {@link Two}.
      * @param [parameters.domElement] - The `<svg />` to draw to. If none given a new one will be constructed.
      */
-    class SVGRenderer extends Two.Events {
+    class SVGRenderer extends   Events {
         constructor(parameters?: {
             domElement?: Element;
         });
@@ -1866,6 +1834,42 @@ declare namespace Two {
     }
 }
 
+/**
+ * Object inherited by many Two.js objects in order to facilitate custom events.
+ */
+ declare class Events {
+    /**
+     * Call to add a listener to a specific event name.
+     * @param [name] - The name of the event to bind a function to.
+     * @param [handler] - The function to be invoked when the event is dispatched.
+     */
+    on(name?: string, handler?: (...params: any[]) => any): void;
+    /**
+     * Call to remove listeners from a specific event. If only `name` is passed then all the handlers attached to that `name` will be removed. If no arguments are passed then all handlers for every event on the obejct are removed.
+     * @param [name] - The name of the event intended to be removed.
+     * @param [handler] - The handler intended to be reomved.
+     */
+    off(name?: string, handler?: (...params: any[]) => any): void;
+    /**
+     * Call to trigger a custom event. Any additional arguments passed after the name will be passed along to the attached handlers.
+     * @param name - The name of the event to dispatch.
+     * @param arguments - Anything can be passed after the name and those will be passed on to handlers attached to the event in the order they are passed.
+     */
+    trigger(name: string, arguments: any): void;
+    /**
+     * @property undefined - Object of different types of Two.js specific events.
+     */
+    static Types: {};
+    /**
+     * Alias for {@link Events.on}.
+     */
+    static bind(): void;
+    /**
+     * Alias for {@link Events.off}.
+     */
+    static unbind(): void;
+}
+
 /**
  * The entrypoint for Two.js. Instantiate a `new Two` in order to setup a scene to render to. `Two` is also the publicly accessible namespace that all other sub-classes, functions, and utilities attach to.
  * @param [options.fullscreen = false] - Set to `true` to automatically make the stage adapt to the width and height of the parent document. This parameter overrides `width` and `height` parameters if set to `true`. This overrides `options.fitted` as well.
@@ -1876,7 +1880,7 @@ declare namespace Two {
  * @param [options.autostart = false] - Set to `true` to add the instance to draw on `requestAnimationFrame`. This is a convenient substitute for {@link Two#play}.
  * @param [options.domElement] - The canvas or SVG element to draw into. This overrides the `options.type` argument.
  */
-declare class Two extends Two.Events {
+declare class Two extends Events {
     constructor(options?: {
         fullscreen?: boolean;
         fitted?: boolean;

[jonobr1]

Thanks, this is helpful to see. As I get further into understanding the type declaration, I think it makes sense to defer to @types/two.js until Two.js is formally setup with more ES6 features. What's difficult about generating this is that we're trying to use jsdocs documentation as the source for type declarations instead of the code. Once the code is more understandable to TypeScript, the types declarations should be a lot easier to generate. Happy to hear everyone else's thoughts on this matter.

[aaron-harvey]

I think that makes sense. The auto generated doc types are close, but need some gaps closed - it would be easier to offload that responsibility to @types until tsc can understand the code base well enough to output the d.ts.

[nmay231]

think it makes sense to defer to @types/two.js until Two.js is formally setup with more ES6 features.

Also agreed. At the moment though, typescript uses the types generated in v0.7.5 even if @types/two is installed. And so upgrading from v0.7.4 breaks existing typescript code.

Once the code is more understandable to TypeScript, the types declarations should be a lot easier to generate.

One aspect I think would help a lot is to convert raw prototypical classes to use the class-keyword syntax. Typescript does a good job of picking up attributes set using this.attr in the constructor and other methods.

[jonobr1]

v0.7.3 explored types generated using tsc and v0.7.4 used the current implementation of jsdocs to types, but because the location of the types.d.ts file changed and I forgot to change the location in the package.json, it wasn't included in that package. I'll release a new version today that has these types removed so that people can continue to use @types/two.

[CaelanStewart]

Currently, I'm only able to import 0.7.4 with TypeScript.

I'm unable to import two.js@0.7.5 (or 0.7.6) even with @types/two.js installed.

I have other packages with their own @types/... that are working fine.

With version >= 0.7.5:

TS2306: File '/Users/redacted/node_modules/two.js/types.d.ts' is not a module

Let me know if I'm missing something, or if you'd like to know anything specific about the setup.

[jonobr1]

Yeah, I removed those types because I'm in the middle of porting all the modules to ES6 classes. This will yield better types inference. I must've mistakenly kept the package.json types file reference even though I removed it from the package. Sorry about that! I'll look into that today.

[jonobr1]

@CaelanStewart, the latest on npm should have a placeholder module declaration for Two.js so that you can at least load the latest version of Two.js into a TypeScript project. This will have to do until I'm able to finish porting everything to ES6 class declarations which will make the TypeScript declarations more accurate through TypeScript's own types generation script.

Apologies to all for the delay. I'm not a TypeScript user, so this has been a crash-course for me on TypeScript.

[CaelanStewart]

@jonobr1 – That's great! Thank you for updating us.

[jonobr1]

This ES6 branch is nearly done, but is held back by a couple of errors in the TypeScript types generation script. The two major roadblocks that I'm stuck on are:

1. There is an error with render being private in /src/renderers/webgl.js that I can't seem to diagnose. See below:
2. Default export of Two.js when it's not a namespace (I believe, is throwing errors). See below:

webgl.js error:

> @jonobr1/two.js@0.7.14 types /two.js
> tsc --outFile types.d.ts

src/renderers/webgl.js:1:1 - error TS9005: Declaration emit for this file requires using private name 'render'. An explicit type annotation may unblock declaration emit.

1 import { Commands } from '../utils/path-commands.js';
  ~~~~~~


Found 1 error.

Two.js error:

Class static side 'typeof Two' incorrectly extends base class static side 'typeof Events'. Types of property 'Types' are incompatible. Type '{ webgl: string; svg: string; canvas: string; }' is missing the following properties from type '{ play: string; pause: string; update: string; render: string; resize: string; change: string; remove: string; insert: string; order: string; load: string; }': play, pause, update, render, and 6 more.

If you check out the ES6 branch and run npm run types after npm installing the dependencies you'll see the first error which halts the rest of the types being generated. The second error comes up after types are generated and you've commented out /src/renderers/webgl.js from being imported into the main Two.js source file /src/two.js.

If you anyone has input on this I'm all ears 😸

[aaron-harvey]

There is an error with render being private in /src/renderers/webgl.js that I can't seem to diagnose.

There is something wonky in the webgl.group.render method that is causing the type checker to fail. I wasn't able to track down the exact cause, but I did manage to satisfy the type checker adding the following type hint:

diff --git a/src/renderers/webgl.js b/src/renderers/webgl.js
index 462534b3971239532aa3fb129a137e5596e3222a..9f9d71621b88966e8bd77308af48539f1567736a 100644
--- a/src/renderers/webgl.js
+++ b/src/renderers/webgl.js
@@ -71,6 +71,7 @@ const webgl = {
       }
     },
 
+    /** @type {(gl: any, programs: any) => any} */
     render: function(gl, programs) {
 
       if (!this._visible) {

[jonobr1]

Amazing, thank you @aaron-harvey. I was able to resolve the other issue as well (there was a conflict of statically assigned properties, so I moved them). I'll try to spend some more time looking into why the group.render method triggers this. The other renderers have the same format for defining render calls, so it's surprising that either the other renderers also don't trigger an error or that the webgl renderer does.

Also, to set expectations:

I finished writing the ES6 branch and with these changes the TypeScript Declarations will be more accurate. But, the tests are failing, so I still need to remedy those before publishing this. Baby steps!

[jonobr1]

For those interested, the WIP types now live here: https://github.com/jonobr1/two.js/blob/0b86fe087ef7f23a03bac828ea26d3e14712c83d/types.d.ts

If you happen to drop it in when developing and you see something, I'd love to get your feedback. Thanks!

[jonobr1]

Small update (2 years in the making), the ES6 branch is now passing all tests: https://github.com/jonobr1/two.js/tree/es6

Gonna give it a couple of days before merging this into dev and updating the node package. This is your time to chime in!

[jonobr1]

Hopefully, v0.8.0 fixes many of your issues. It's available now through npm.

[drbolsen]

Got the same issue with the latest package 0.8.0

import Two from "two.js"

"two.js/types.d.ts' is not a module."

[atmosbear]

Got the same issue with the latest package 0.8.0

import Two from "two.js"

"two.js/types.d.ts' is not a module."

I had this too and it was driving me crazy. I tried all sorts of things but what eventually worked for me was deleting my node_modules folder, not using the minified version of Two, because it's easier to install through NPM, and doing 'npm install two.js', NOT 'npm install two'.