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 bindings? #18

Open
mattes3 opened this issue Jun 11, 2019 · 18 comments
Open

Typescript bindings? #18

mattes3 opened this issue Jun 11, 2019 · 18 comments

Comments

@mattes3
Copy link

mattes3 commented Jun 11, 2019

Hi,

is there a *.d.ts file to be able to consume this nice library from Typescript?

Best...
Matthias

@IceCreamYou
Copy link
Owner

Unfortunately not, but I'd be happy to merge a contribution!

@mattes3
Copy link
Author

mattes3 commented Jun 12, 2019

I've just seen that THREE.Terrain does not use any module bundler (e.g. rollup.js, like three.js itself does). This prevents the code from being used in a larger, modularised, web application like mine that uses the import and export statements in the Javascript source code.

Could you turn your *.js files into ES modules, please? I could then contribute Typescript typings as a PR!

You can configure the module bundler so that it generates modules that can be used directly in the browser and can also be imported into a larger, modularised web app.

Here is the info that you might need for that:

@IceCreamYou
Copy link
Owner

The way this library is currently written, you wouldn't import it as a module. The typings would look something like this:

declare module 'THREE' {
	export class Terrain {
		// ...
	}
}

...and then you'd just use it as THREE.Terrain. However the THREE.Terrain code also needs to know where THREE lives, and since it's a library that supports non-ES6 code it needs to support doing that in multiple ways, which would involve significant changes. (This is roughly the same reason why Three.js's own examples are being rewritten to be ES6-compatible and put in a separate folder from the non-ES6-compatible versions; and it's easier there, because the examples know where they live relative to three.js.) The easiest solution is to keep THREE as a global. In that setup, in TypeScript you'd "import" three like this:

declare var THREE: typeof import('three');

However, doing that gives up some advantages of using Three.js in a more modular fashion.

Changing THREE.Terrain to avoid these problems and support multiple build targets would be a larger rewrite than I have time for, unfortunately, and would likely involve breaking changes. That said, I'd love for someone else to take over the library and do that. The library's API would benefit from some cleanup anyway. It'd be nice to make most of the library know nothing about Three.js and just have an adapter layer that can work with Three.js entities.

@mattes3
Copy link
Author

mattes3 commented Jun 13, 2019

Maybe we could make a "deal". 😄

Starting in the 2nd week of July, I could take over the library for a while and refactor it towards modules (I will try to keep it possible that people can include it via <script> tags in their HTML pages, too).

I would turn the code itself to Typescript and I would make it transpile to ES5 so that it runs within non-ES6 environments, too. The Typescript compiler would automatically create the typings, then.

However (and this is why I talked about a "deal"), I would like to have support from you from time to time when I get stuck, and I'd like to learn from you about procedural terrain generation in general.

How does that sound?

If you agree, you can add me as a committer to THREE.Terrain. Would be exciting.

@IceCreamYou
Copy link
Owner

Starting in the 2nd week of July, I could take over the library for a while and refactor it towards modules (I will try to keep it possible that people can include it via <script> tags in their HTML pages, too).

I would turn the code itself to Typescript and I would make it transpile to ES5 so that it runs within non-ES6 environments, too. The Typescript compiler would automatically create the typings, then.

Sounds great!

I would like to have support from you from time to time when I get stuck, and I'd like to learn from you about procedural terrain generation in general.

Happy to do this.

If you agree, you can add me as a committer to THREE.Terrain.

Since I haven't reviewed any code from you before, the way I'd like to do this is to start out using the PR system. If you put up PRs, I will review them. If I see that you are making positive contributions to the project then I will add you as a committer. Does that sound reasonable?

@mattes3
Copy link
Author

mattes3 commented Jun 13, 2019

Hi Isaac,

...Sounds great!

Cool.

...Happy to do this.

Even cooler.

Since I haven't reviewed any code from you before, the way I'd like to do this is to start out using the PR system. If you put up PRs, I will review them. If I see that you are making positive contributions to the project then I will add you as a committer. Does that sound reasonable?

Absolutely! I'll clone the repo, set up the build, and I will add a PR as soon as it adds value. I'll be able to start in July with this.

Cheers,
Matthias

@IceCreamYou
Copy link
Owner

Awesome 👍

@mattes3
Copy link
Author

mattes3 commented Jul 8, 2019

Hi Isaac,

you can have a look at my first Typescript-based version of THREE.Terrain: https://github.com/mattes3/THREE.Terrain

I managed to port it such that there are no breaking changes to the API, except exactly one: It is now THREE.Terrain.Terrain() instead of THREE.Terrain(). Everything else remained the same.

There are now three different kinds of output: One file with a UMD module to run inside a browser, one file with a CommonJS module, and one file with an ES module.

Please review this solution, using the following steps:

git clone https://github.com/mattes3/THREE.Terrain
cd THREE.Terrain
yarn
yarn build
ls dist/
yarn serve

I will now continue to work on this. The next step is to consume the new CommonJS module inside a Typescript project. This will show whether the *.d.ts files work OK or not (not sure, yet).

Curious what you will say...
Matthias

@IceCreamYou
Copy link
Owner

Cool! I just want to acknowledge that I have seen this and will take a look, but it may take me a bit considering the scope.

@mattes3
Copy link
Author

mattes3 commented Jul 11, 2019

I forgot to export the Worley function, so I added some forgotten exports today.

Your demo (index.html/index.js) now works with the TS code as beautifully as before with the JS code.

@mattes3
Copy link
Author

mattes3 commented Jul 12, 2019

OK, now I can confirm that the code works both in a browser and inside a Typescript app. I'll now create a pull request so that you can see the exact changes that I made.

@IceCreamYou
Copy link
Owner

As an update, I'm about halfway through reviewing. Only a couple minor comments on the code as of yet. I'm also working through understanding a couple structural things. Good work so far!

@mattes3
Copy link
Author

mattes3 commented Jul 15, 2019

Thanks, Isaac, for the update!

@dustinlacewell
Copy link

Any chance for this?

@oveddan
Copy link

oveddan commented May 4, 2021

I'd love this as well. Until this is merged @mattes3 could you please publish your repo in npm so we can use it right now?

@linonetwo
Copy link

Written in ts will make the code easier to learn!

@k2xl
Copy link

k2xl commented Mar 24, 2022

@mattes3 are you able to get this in npm?

@mattes3
Copy link
Author

mattes3 commented Mar 25, 2022

No, sorry, got no time anymore for this project. Someone else has to take this over.

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

6 participants