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

Update package.json exports paths #23354

Merged
merged 1 commit into from
Jan 27, 2022
Merged

Update package.json exports paths #23354

merged 1 commit into from
Jan 27, 2022

Conversation

brunosimon
Copy link
Contributor

Related issue: -

Description

Update the exports paths to include files located in the ./examples/ folder like the JSON fonts.

Related issue: -

**Description**

Update the export paths to include files located in the `./examples/` folder in order to include JSON fonts.
@mrdoob
Copy link
Owner

mrdoob commented Jan 27, 2022

@mrdoob mrdoob added this to the r137 milestone Jan 27, 2022
@marcofugaro
Copy link
Contributor

marcofugaro commented Jan 27, 2022

Looks good!

As an alternative we could add this to the exports list:

"./examples/fonts/*": "./examples/fonts/*",

but maybe people are importing other stuff other than jsm and fonts from the examples/ folder?

@mrdoob
Copy link
Owner

mrdoob commented Jan 27, 2022

Adding only the examples/fonts folder sounds better indeed.
I'll merge and tweak.

@mrdoob mrdoob merged commit 34bbcc4 into mrdoob:master Jan 27, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jan 27, 2022

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Jan 27, 2022

3d0c8df

@marcofugaro
Copy link
Contributor

@mrdoob leaving this for the future (when you decide to), we could also do this for the npm users like Bruno:

import { OrbitControls } from 'three/addons';

import { helvetiker_regular } from 'three/fonts';

The current three/examples/jsm/... and three/examples/fonts/... imports would still work fine, but npm users could leaverage tree-shaking to import only OrbitControls and not the whole examples/jsm folder.

@mrdoob
Copy link
Owner

mrdoob commented Jan 27, 2022

Not sure about fonts...

Feels like three would then become the new OS built-in fonts 😅

As per three/addons... Wouldn't it be this instead?

import { OrbitControls } from 'three/addons/controls/OrbitControls';

Or is there a way to "flatten" all the files in the three/addons?

@marcofugaro
Copy link
Contributor

Yeah, let's not tell people to import fonts from three hahaha

Or is there a way to "flatten" all the files in the three/addons?

Yes, simply by adding a file similar to src/Three.js 😇

npm users would still import only the things they need thanks to tree-shaking..

@mrdoob
Copy link
Owner

mrdoob commented Jan 27, 2022

Yes, simply by adding a file similar to src/Three.js 😇

D'oh!

@mrdoob
Copy link
Owner

mrdoob commented Jan 27, 2022

That sounds promising indeed. Do you want to get a PR going with that?

@marcofugaro
Copy link
Contributor

Sure! Let's see what people say...

@mrdoob
Copy link
Owner

mrdoob commented Jan 28, 2022

So wait...

Is there a way to achieve something like this?

<script type="importmap">
	{
		"imports": {
			"three": "https://unpkg.com/[email protected]/",
		}
	}
</script>
    
<script type="module">
      
	import * as THREE from 'three';
	import { GLTFLoader } from 'three/addons/loaders/GLTFLoader.js';
	import { VRButton } from 'three/addons/webxr/VRButton.js';

	...
</script>

@marcofugaro
Copy link
Contributor

I assume you mean without renaming the examples/jsm folder.

  • unpkg.com: no, it wouldn't work, unpkg only serves the actual files
  • cdn.skypack.dev: yes, skypack has support for this, but only if they fix the issue with the current three.js release (skypackjs/skypack-cdn#261)

@marcofugaro
Copy link
Contributor

marcofugaro commented Jan 28, 2022

oh wait, that is not a valid import map, import maps do not automatically work like node, but some improvements are being discussed,

so with the current spec, it would look like this:

I was wrong, see #23368 (comment)

@LeviPesin
Copy link
Contributor

I think we should either reconsider exporting the fonts or at least export them as three/fonts (I'd prefer the former).

@mrdoob
Copy link
Owner

mrdoob commented Nov 7, 2022

@LeviPesin Context? 🤔

@LeviPesin
Copy link
Contributor

LeviPesin commented Nov 7, 2022

Just thinking from one of the points of #24595 and 3d0c8df...

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.

4 participants