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

Bun build support #7242

Merged
merged 5 commits into from
Dec 29, 2024
Merged

Bun build support #7242

merged 5 commits into from
Dec 29, 2024

Conversation

kpal81xd
Copy link
Contributor

@kpal81xd kpal81xd commented Dec 27, 2024

This PR stems from attempting to build the engine from source as a submodule within a bun-based project. Ideally, the engine should be able to be built in bun successfully as a submodule without conflicting with the parent project node module types.

Overview

  • Fixes type searching to be limited to local node modules (stops looking in folders above if engine is submodule)

@kpal81xd kpal81xd marked this pull request as ready for review December 27, 2024 18:06
@kpal81xd kpal81xd self-assigned this Dec 27, 2024
@willeastcott
Copy link
Contributor

Can you add some context about the motivation for this PR please?

@willeastcott
Copy link
Contributor

So what's the Bun lockfile? How and when does it get updated?

@kpal81xd
Copy link
Contributor Author

So what's the Bun lockfile? How and when does it get updated?

So the bun lockfile works the same as package-lock.json but instead of being a json object its a binary.

@willeastcott
Copy link
Contributor

So what's the Bun lockfile? How and when does it get updated?

So the bun lockfile works the same as package-lock.json but instead of being a json object its a binary.

I expressed some concern to ChatGPT for its take and it chimed with what I was thinking:

Your inclination not to commit a bun.lockb file to the repository is valid, especially given the PlayCanvas Engine's primary reliance on npm as its package manager. Including a lock file for a package manager that the repository doesn't officially support can introduce confusion, increase maintenance overhead, and potentially give the impression that bun is officially supported when it is not.

If you're using a submodule, can't you use a local override or generate a file into your submodule with a pre-build step or something?

@kpal81xd
Copy link
Contributor Author

So what's the Bun lockfile? How and when does it get updated?

So the bun lockfile works the same as package-lock.json but instead of being a json object its a binary.

I expressed some concern to ChatGPT for its take and it chimed with what I was thinking:

Your inclination not to commit a bun.lockb file to the repository is valid, especially given the PlayCanvas Engine's primary reliance on npm as its package manager. Including a lock file for a package manager that the repository doesn't officially support can introduce confusion, increase maintenance overhead, and potentially give the impression that bun is officially supported when it is not.

If you're using a submodule, can't you use a local override or generate a file into your submodule with a pre-build step or something?

So I've tested building the engine with bun and that works fine too with the same package lock. Yes you do not need to have bun lock file but for installation performance it's advised to have it by Bun themselves.

As for the using the engine as a sub module the types scoping is required regardless as types of the sub module will conflict is you are using a different runtime in the engine to your parent project.

@willeastcott
Copy link
Contributor

Can we compromise on the types update then and back out the bun lockfile for now?

@kpal81xd
Copy link
Contributor Author

Can we compromise on the types update then and back out the bun lockfile for now?

Yep that works for me

@kpal81xd kpal81xd merged commit b8f4cc5 into main Dec 29, 2024
7 checks passed
@kpal81xd kpal81xd deleted the bun branch December 29, 2024 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants