Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript: module augmentation impossible #19468

Closed
willstott101 opened this issue May 26, 2020 · 5 comments
Closed

TypeScript: module augmentation impossible #19468

willstott101 opened this issue May 26, 2020 · 5 comments

Comments

@willstott101
Copy link
Contributor

willstott101 commented May 26, 2020

Due to typescript bug microsoft/TypeScript#18877

It is impossible to extend THREE.js class prototypes if they're imported via Three.d.ts (i.e. import { Object3D } from "three";) using the method described here: https://www.typescriptlang.org/docs/handbook/declaration-merging.html#module-augmentation

This has been brought up before, but as a a support question (#13425). I believe this is actually something that needs resolving through changes to Three, so have re-opened.

It looks as though the best solution to this as far as I can tell, is exporting an explicit list of names from Three.d.ts like: export { Object3DIdCount, Object3D } from './core/Object3D'; (which does work fine if I modify the .d.ts in my node_modules)

Unfortunately this bug in tsc was closed as a design limitation: microsoft/TypeScript#9532

I want to know if you would be on board with exporting every name explicitly from Three.d.ts, and how you would prefer to test that it works. I'm then happy to write a PR :)

My suggestions are making it fully automatic, as a part of:

EDIT: Changed to clarify the problem.

@willstott101
Copy link
Contributor Author

Also see: microsoft/TypeScript#18877

@donmccurdy
Copy link
Collaborator

donmccurdy commented May 26, 2020

I don't think I understand your question. What problem are you trying to solve by patching the Object3D prototype, and what specific code changes do you need here to make that work? Would every type definition used anywhere in the project need to be exported from the top-level Three.d.ts definitions?

afaik there are no test or scripts making sure the current Three.d.ts is up to date atm, but I may have missed something.

The npm run test-lint and npm run lint-examples scripts use tsc to check the type definitions against examples and unit tests. This doesn't guarantee they're completely up to date, but it helps.

@willstott101
Copy link
Contributor Author

willstott101 commented May 26, 2020

The exact thing that I want to make work is the below:

import { Object3D } from "three";
import { Entity } from "./Entity";

declare module "three" {
  export interface Object3D {
    entity?: Entity;
    findOwningEntity: () => Entity | "world";
  }
}

Object3D.prototype.findOwningEntity = function () {
  if (this.entity) return this.entity;
  let p = this.parent;
  if (p) return p.findOwningEntity();
  return "world";
};

Where I just want to be able to store extra information on every Object3D in my scene.

I've found this a very useful pattern in the past (in pure JS), often when trying to find some logical component which is responsible for something in the scene graph during a raycast query.

Unfortunately due to typescript bug microsoft/TypeScript#18877 this is impossible when Three.d.ts exports using export * from './core/Object3D';

However if Three.d.ts names the exports explicitly like export { Object3DIdCount, Object3D } from './core/Object3D; then the augmentation to the Object3D class prototype compiles fine.

For now I've just worked around it by making pure-functions which take an Object3D as an argument and use as any but this is really awkward as an API - vs adding to Object3D's prototype.

@willstott101
Copy link
Contributor Author

I've changed the original post considerably to make it more clear.

@donmccurdy
Copy link
Collaborator

Thanks, that is clearer yes. I'm still confused about why this is the case in TypeScript, but perhaps that is beside the point.

Unfortunately, I don't think we are able to maintain the more explicit exports in the way you suggest. That creates a fair bit more complexity managing files that are already quite difficult to manage and test.

Could I suggest that you consider other options here? For example:

A. Extend Object3D with inheritance, rather than modifying the prototype.
B. Attach extra methods to a shared ObjectUtils class, or perhaps object.userData (which is not shared between instances, though).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants