playcanvas / engine

JavaScript game engine built on WebGL, WebGPU, WebXR and glTF
https://playcanvas.com
MIT License
9.55k stars 1.34k forks source link

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

Closed epreston closed 11 months ago

epreston commented 1 year ago

Build directory is not cleared before rebuilding all targets.

Normally, the expectation is that:

Options:

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 commented 1 year ago

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

mvaligursky commented 11 months ago

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

epreston commented 11 months ago

On the way.

epreston commented 11 months ago

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

kungfooman commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

On the way.

epreston commented 11 months ago

@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 commented 11 months ago

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

epreston commented 11 months ago

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 commented 11 months ago

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