toji / gl-matrix

Javascript Matrix and Vector library for High Performance WebGL apps
glmatrix.net
MIT License
5.37k stars 721 forks source link

Support TS in sub packages #390

Open TrySound opened 4 years ago

TrySound commented 4 years ago

Ref https://github.com/toji/gl-matrix/issues/389

Emitted all types to directory instead of single file. This way each sub package can specify own "types" field.

Though there are problems

I solved by manual prepending import from types

import { mat2, mat2d, mat3, mat4, quat, quat2, vec2, vec3, vec4, ReadonlyMat2, ReadonlyMat2d, ReadonlyMat3, ReadonlyMat4, ReadonlyQuat, ReadonlyQuat2, ReadonlyVec2, ReadonlyVec3, ReadonlyVec4 } from './types';
/**
 * Quaternion
 * @module quat
 */
/**
 * Creates a new identity quat
 *
 * @returns {quat} a new quaternion
 */
export function create(): quat;
import { ..., vec2, ... } from './types';
export const fromValues: typeof vec4.fromValues;
hustcc commented 4 years ago

Maybe this the minimum cost plan for sub-packages type define.

nit: But because the npm package files are generated by script, so we can only test the type define locally, this can easily lead to package quality problems. So test case for type define is suggested.

  1. build npm package
  2. the test file(.ts) should import dist folder
  3. tsc the test files, check whether there are type define errors
stefnotch commented 4 years ago

I've been looking at this again and it seems that if Rollup implements this https://github.com/rollup/rollup/issues/2201 (Object shape tree-shaking), we wouldn't need the sub packages for tree-shaking anymore. Is that correct?

TrySound commented 4 years ago

I didn't dig in this proposal. Can't say anything.

stefnotch commented 4 years ago

I think this would be the relevant webpack issue https://github.com/webpack/webpack/issues/8057

0ctothorp commented 4 years ago

Hey, I just tried this in my project and it works great. Basically I just wanted this kind of imports to work well in typescript and this PR seems to do it just fine!

import { fromTranslation } from 'gl-matrix/mat4';

I would love to help push this forward if it's possible, but am not sure what problems are preventing it from being merged.

stefnotch commented 4 years ago

Good question! The main thing preventing me from merging is that I currently don't have enough time to sort out the issues. I'm rather preoccupied with other projects.

donmccurdy commented 3 years ago

I think there's an easier way to solve this, by appending a few extra lines to the existing type definitions:

declare module 'gl-matrix/vec4' {
    import { vec4 } from 'gl-matrix';
    export = vec4;
}

declare module 'gl-matrix/vec3' {
    import { vec3 } from 'gl-matrix';
    export = vec3;
}

declare module 'gl-matrix/vec2' {
    import { vec2 } from 'gl-matrix';
    export = vec2;
}

declare module 'gl-matrix/mat4' {
    import { mat4 } from 'gl-matrix';
    export = mat4;
}

declare module 'gl-matrix/mat3' {
    import { mat3 } from 'gl-matrix';
    export = mat3;
}

^I've been using those in a global.d.ts file in my own project, but would be happy to submit them here if that sounds OK? Or have I missed part of the intent for this PR?

stefnotch commented 3 years ago

@donmccurdy Honestly, at this point, any solution that works would be super appreciated. So, a clear 👍 from my side.

donmccurdy commented 3 years ago

Hm, I tried putting that code (which works in a consuming project) into gl-matrix and there are some issues. TypeScript still needs separate files for each subpackage, and those subpackages must be located at the project root, e.g. ./mat4.d.ts or ./mat4/index.d.ts (see https://github.com/microsoft/TypeScript/issues/8305). If polluting the project root is OK we could go ahead with that, or even generate and publish those files but omit them from versioning with .gitignore. Presumably that limitation applies to this PR, too.

If we want to avoid those root-level files or directories, TypeScript may begin supporting package exports around v4.4, and this would provide more flexible, explicit mappings to each entrypoint. Perhaps waiting for v4.4 is the way to go..