-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add Engine ESM build #6073
Add Engine ESM build #6073
Conversation
Very nice, people needed this for quite some time - unbundled ESM builds were causing:
|
Unfortunately even the ES Modules build includes these polyfils which are likely redundant |
utils/rollup-build-target.mjs
Outdated
@@ -104,7 +105,7 @@ function buildTarget(buildType, moduleFormat, input = 'src/index.js', buildDir = | |||
|
|||
const outputExtension = { | |||
es5: '.js', | |||
es6: '.mjs' | |||
es6: shouldBundle ? '.esm.js' : '.mjs' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current file extension scheme .js
means UMD, shouldn't we keep .mjs
? Maybe something like playcanvas.bundled.mjs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah there's no easy answer here. playcanvas.mjs
is the most obvious, and consistent with the UMD playcanvas.js
build, but the .mjs
suffix is currently pointing to a directory, and so I wonder if that build actually needs to be renamed
Should we have other flavours of this bundle? At least debug, performance, and release (normal). |
Yep there's the same outputs, dbg, prf and min |
Had a little discussion among the team and we'd vote for: And of course .dgb, .min, .profile version of each. |
We also need to change in that case: Line 21 in 6314936
|
Love this. Done! Assuming the unbundled 'playcanvas' dir doesn't need to have 'playcanvas.prf', 'playcanvas.min', 'playcanvas.dbg' variants |
Yep agreed. Have updated |
I think we will need variants... @mvaligursky ? |
What would be the naming convention on these folders? Having the dot suffix doesn't feel right. kebab case 'playcanvas-dbg/index.js' might work if we do
|
If we expect people to use the unbundled versions, we need to provide the variants, as they cannot easily replicate our processes to strip out debug info, and they need debug version at the very least. |
For consistency naming the folders |
Though in reality the |
Have added unbundled es6 variants excluding 'min' @slimbuck @mvaligursky |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<3
This PR adds an additional ES6 bundle
playcanvas.esm.js
as a build output. It uses the same ES6 build configuration as the unbundledplaycanvas.mjs
but instead simply outputs to a single output.We plan on upgrading the launcher and exported projects to use this ES6 build to support ESM Scripts.
I confirm I have read the contributing guidelines and signed the Contributor License Agreement.