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

Examples: Clean up #25057

Merged
merged 5 commits into from
Dec 2, 2022
Merged

Examples: Clean up #25057

merged 5 commits into from
Dec 2, 2022

Conversation

LeviPesin
Copy link
Contributor

Description

Some clean up based on the output of #25056.

return item != undefined && item != null;
return item !== undefined && item !== null;
Copy link
Collaborator

@gkjohnson gkjohnson Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed in the original repository.

cc @agviegas - does it make sense to use something like "unpkg" and import maps in the example rather than copying the build into three.js examples?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Seems like the loader in the repo is a bit old now too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, that's right! The project is growing a lot, and chances are that next year it will take off. To avoid bloating three.js and to have a single source of truth, maybe it's a better idea to keep it outside of three.js, like three-mesh-bvh? 🤔 What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's best - the three.js examples are still a good way to enable people to discover the external projects, though. The ifc js project can be modified to use an import map like the three-mesh-bvh raycasting example here:

<script type="importmap">
{
"imports": {
"three": "../build/three.module.js",
"three/addons/": "./jsm/",
"three-mesh-bvh": "https://unpkg.com/three-mesh-bvh@^0.5.10/build/index.module.js"
}
}
</script>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Should I update the example?

Copy link
Collaborator

@gkjohnson gkjohnson Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's my opinion - then we can delete the ifc.js files from the project and direct users to use the canonical, up to date repo.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agviegas

Ok! Should I update the example?

Yes please 🙏

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrdoob got it!

@mrdoob mrdoob added this to the r148 milestone Dec 2, 2022
@mrdoob mrdoob merged commit 5368e6a into mrdoob:dev Dec 2, 2022
@LeviPesin LeviPesin deleted the cleanup branch December 2, 2022 07:02
@agviegas
Copy link
Contributor

agviegas commented Feb 5, 2023

Done: #25440 🙂

@harryhjsh harryhjsh mentioned this pull request Apr 9, 2024
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