Skip to content

Conversation

@sunrabbit123
Copy link
Member

This pull request simplifies the build process by removing a custom build script and replacing it with a more efficient workspace-based build command using pnpm.

Build process simplification:

  • Removed the custom deploy/build.js script, which handled package builds manually by iterating through packages and running build commands.
  • Updated the build script in package.json to use pnpm -r build, leveraging pnpm's recursive workspace feature for building all packages.
],
"scripts": {
"build": "node deploy/build",
"build": "pnpm -r build",
Copy link
Member Author

Choose a reason for hiding this comment

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

but failed with packages/compiler-browser-test

Copy link
Member Author

Choose a reason for hiding this comment

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

and it's code ignore private field
The existing code worked correctly by excluding packages marked as private.

But idk why ignore private package in script

Copy link
Member Author

Choose a reason for hiding this comment

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

what do you think? @Reviewer

@AcrylicShrimp
Copy link
Member

AcrylicShrimp commented May 2, 2025

I don't think we can simply use pnpm -r build to build all packages at once. This is because:

  1. Our project contains some packages that SHOULD NOT be part of the build process. These packages are marked as private currently. Maybe we can exclude them from the workspace members.
  2. Some packages depend on other packages. The build order matters.

The second one will be problematic. Do you have any idea to keep the build order safe?

@sunrabbit123
Copy link
Member Author

  1. yeah, i understand it and mentiond above, it's sad news

  2. we can use build script and workspace dependent, but basically pnpm workspace use topology sort (https://pnpm.io/ko/next/cli/recursive#--no-sort)
    so it's possible if we can simply configure a build pipeline.

But we can't solve the number 1
I'm asking for your opinion without working on it.

@AcrylicShrimp
Copy link
Member

Um, than why don't you remove "problematic" packages from the workspace and try again? I think the "private" ones (including packages/compiler-browser-test) can be removed without any issue.

But you might want to make some packages "public" again (packages/compiler-bundle for example) because they must be part of the build process; some packages depend on them.

@sunrabbit123
Copy link
Member Author

The functionality itself can be made to work one way or another.
However, the main point in item 1 is as follows:

The purpose of the private field is to prevent publishing.

Therefore, since a package with a private field that is set contrary to the package manager's intended use cannot be ignored at the pnpm level,

we must instead modify the build process to include all packages. This raises the question of whether such a change would go against the original intent of the person who designed the package structure.

This is not about an error or malfunction—I'm simply asking for an opinion.

If the current private setting is intentional, then the PR loses its value, and there’s no reason to proceed with it.
As for refactoring the build process, I haven’t yet started thinking or designing it, because there are some things that must be addressed beforehand.

thank you for reading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants