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

Polyfills should be a build step only #6072

Closed
marklundin opened this issue Feb 21, 2024 · 8 comments
Closed

Polyfills should be a build step only #6072

marklundin opened this issue Feb 21, 2024 · 8 comments

Comments

@marklundin
Copy link
Member

The engine currently ships with a load of polyfills which are automatically imported in a build whether you need them or not. Consequently building against a modern target like ES2020 inlines these polyfils when they're actually not required. In addition, if playcanvas is included in a project with it's own build step + polyfills, these would likely clash.

Ideally polyfills should be part of the build step and not the source code. Using core-js with babel-preset-env or similar, only the required polyfills are chosen based on the chosen output target

@marklundin marklundin changed the title Polyfill should be a buld step Polyfill should be a build step Feb 21, 2024
@marklundin marklundin changed the title Polyfill should be a build step Polyfills should be a build step only Feb 21, 2024
@mvaligursky
Copy link
Contributor

Perhaps we should only add them when building ES5 targets, but not when building ESM?

@marklundin
Copy link
Member Author

Perhaps we should only add them when building ES5 targets, but not when building ESM?

We also don't include a polyfill for Object.entries() which is partially responsible for #6011. So including it as part of the build would catch whenever we introduced a new language feature an polyfill for it

@marklundin
Copy link
Member Author

For now though, you're probably right @mvaligursky, we can just add a conditional flag that omits from the ES6 build

@slimbuck
Copy link
Member

Wouldn't it be easier to change the engine compile process to translate output ESM build to UMD and have that step add the polyfills?

@marklundin
Copy link
Member Author

core-js does this. However some initial tests show it's quite aggressive. For example including new Float32Array() adds polyfills for all methods, even those methods are never called. I imagine this is by design (maybe some of the polyfills depend on each other), but as our build UMD target is pretty old, core-js is adding around 160Kb of code, just for TypedArray.

Auto-polyfilling definitely seems like the right way to go, but it needs some more investigation and optimisation. Either way, as #6011 shows, we don't have a polyfill for Object.entries, which is used in a few places, so the current UMD build has not worked with those code paths on pre Chrome 54 era browsers.

@mvaligursky
Copy link
Contributor

Considering we're hoping to deprecate the IMD build in no too distant future, we might just add that single polyfill missing at the moment and call it a day for now.

@marklundin
Copy link
Member Author

I think that's fine for now, we just exclude these in the ES6 build

@marklundin
Copy link
Member Author

marklundin commented Jul 5, 2024

Closing for now. As of #6079 ESM build omits the polyfills.

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

No branches or pull requests

3 participants