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

Build directory not cleared before build "all" targets #5588

Closed
epreston opened this issue Aug 24, 2023 · 11 comments · Fixed by #5695
Closed

Build directory not cleared before build "all" targets #5588

epreston opened this issue Aug 24, 2023 · 11 comments · Fixed by #5695

Comments

@epreston
Copy link
Contributor

epreston commented Aug 24, 2023

Build directory is not cleared before rebuilding all targets.

Normally, the expectation is that:

  • building one or specific set of targets - do not clear
  • building all targets - rimraf the build dir before building

Options:

  • Can be done via cli before "npm run build" with a rimraf command.
  • Can be done in code in the build script

Example for rollup.config.mjs in the default export

  // if building a specific format, do not remove build.
  if (!targets.length && existsSync(`build`)) {
    await fs.rm(`build`, { recursive: true });
  }

I ran into an issue that confused me when I thought some files were not being built. They are no longer part of the build but were left there from 3 weeks ago. Clearing the directory on full builds will ensure this does not happen again.

It will also ensure extra files are not published to npm bloating the bundle size. For example "common.js" in the shaders directory, removed in a commit a few weeks ago, in a recent publish.

@willeastcott
Copy link
Contributor

The build command should probably build the TypeScript declarations too then.

@mvaligursky
Copy link
Contributor

keen to do all this @epreston ? I'd love to have this.

@epreston
Copy link
Contributor Author

On the way.

@epreston
Copy link
Contributor Author

Ready for review. A little re-org for clarity and the normal clean-up logic on "build all".

@kungfooman
Copy link
Collaborator

The build command should probably build the TypeScript declarations too then.

Fully agree, but we still lack build/playcanvas.d.ts after npm run build now

@mvaligursky
Copy link
Contributor

Yes, I got hit by this yesterday and it took me a while to figure out what is missing. It'd be great to add.

@epreston
Copy link
Contributor Author

epreston commented Oct 6, 2023

On the way.

@epreston
Copy link
Contributor Author

epreston commented Oct 13, 2023

@willeastcott , @mvaligursky , @kungfooman

A command already exists to build types after the library is built.

npm run build:publish

It is defined as:

"build:publish": "npm run build && npm run build:types",

You can be confident that it is clean because the first command purges the directory.

Correction: it does not purge the types directory. There's a misconfiguration / expectation that needs discussion.

@mvaligursky
Copy link
Contributor

Hi interesting .. personally I'd remove the publish target and make the build do this.
@slimbuck @willeastcott - any thoughts?

@mvaligursky mvaligursky reopened this Oct 13, 2023
@epreston
Copy link
Contributor Author

Can I speed up the build 10x first by swapping a few rollup plugins ? Then we add to it ?

The best in class plugin for this at the moment does both at the same time. Worth taking a look at.

@mvaligursky
Copy link
Contributor

Sure, create a separate issue / pr and go for it, keen to see what it is.

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

Successfully merging a pull request may close this issue.

4 participants