Skip to content

Conversation

@hujiulong
Copy link
Contributor

Add types property to indicate the main declaration file.
docs

@hujiulong hujiulong closed this Feb 3, 2019
@hujiulong
Copy link
Contributor Author

The published npm package does not contain the src folder🤔

@hujiulong hujiulong reopened this Feb 3, 2019
@hujiulong
Copy link
Contributor Author

How about we just ignore .js files in the src folder to reduce the size of npm package?

@donmccurdy
Copy link
Collaborator

@mrdoob we wanted to allow people to import from the src/ dir anyway right? May as well start including it I think.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 4, 2019

Just for my understanding: package.json already refers to three.module.js. Wouldn't it a bit redundant if users can import classes from this module and the source files.

@donmccurdy
Copy link
Collaborator

Completely redundant, yes. But at least the source files themselves are small, especially compared to examples/models. Part of the idea behind converting the .glsl files to .js was to change the package.json pointer:

  • main -> build/three.min.js
  • module -> src/Three.js

... In theory, this means build systems using ES modules are more likely to tree-shake the library effectively. But at the moment we have more hypotheses about how tree-shaking works than actual evidence... including them in the module without replacing the module entry might be safe way to let users test this in their build systems and get some early results.

All that to say, I'm fine with including the src/ tree in the npm package.

@hujiulong
Copy link
Contributor Author

@mrdoob Hi, So can you merge this PR?

@donmccurdy
Copy link
Collaborator

@LuWangThreekit @Methuselah96 I'm assuming this is the right file to use? We should be able to test this with just npm link three in a local TS project, correct?

@Methuselah96
Copy link
Contributor

Methuselah96 commented Feb 7, 2019

@donmccurdy Yeah, I just tested this locally and it works. There are some things that need to be fixed about the typings (they don't export all of their interfaces (e.g. ExtrudeGeometryOptions, TextGeometryParamters, etc.), they only export the ones that match three.js classes (ExtrudeGeometry, TextGeometry, etc)), but I can fix that in a separate PR. This PR LGTM.

@mrdoob
Copy link
Owner

mrdoob commented Feb 7, 2019

Just for my understanding: package.json already refers to three.module.js. Wouldn't it a bit redundant if users can import classes from this module and the source files.

three.module.js includes Three.Legacy.js and seems like build tools have a hard time to tree-shake all that. When importing directly from src/, build tools seem to produce smaller outputs.

@mrdoob mrdoob added this to the r102 milestone Feb 7, 2019
@mrdoob mrdoob merged commit 854f91b into mrdoob:dev Feb 7, 2019
@mrdoob
Copy link
Owner

mrdoob commented Feb 7, 2019

Thanks!

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

Successfully merging this pull request may close these issues.

5 participants