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

Bundling only the polyfill (no shims) #406

Closed
luisherranz opened this issue Dec 4, 2023 · 11 comments
Closed

Bundling only the polyfill (no shims) #406

luisherranz opened this issue Dec 4, 2023 · 11 comments

Comments

@luisherranz
Copy link

Hey 👋🙂

I'm leading an initiative to add native support for modules and import maps in WordPress, but to do so, WordPress will have to ship a polyfill for some years.

The problem is that, in WordPress, due to its infinite backward compatibility policy, once a feature is shipped, it cannot be removed. If we ship this polyfill with all the optional shims, we won't be able to remove the polyfill in the future because we won't know if there are any plugins out there that are using one of the shims and could break.

We really like this library and the way it works, but is there a way to bundle only the polyfill without any of the shims without having to fork it?

@guybedford
Copy link
Owner

Hi, and happy to help where I can here @luisherranz.

As far as I can tell what you want is just the polyfill features, but without the risk of shim features applying that Wordpress users might rely on downstream causing compatibility problems?

So some kind of "lock down" for es-module-shims itself where it goes into a polyfill mode that can't be hooked?

This does seem to be mostly compatible with how our configuration model works in that the configuration is read immediately on startup and only applies then.

Alternatively we could expose some explicit function like importShim.polyfillLockdown().

Does this sound like the overall type of direction you are after here? If so we can flesh out the design a little further.

@luisherranz
Copy link
Author

luisherranz commented Dec 7, 2023

Yeah, exactly, although it's a bit more difficult because of the way backward compatibility works in WordPress.

  • People could remove the importShim.polyfillLockdown() from their HTML because they can modify the HTML before it's sent to the client.
  • If people remove the importShim.polyfillLockdown() from their HTML, they can use the shim features.
  • If people are using the shim features, WordPress can't remove the polyfill because it might break some sites.
  • Therefore, the polyfill cannot be removed.

So, ideally, we would not ship the shim features at all (if that's possible), or ship a version where they are deactivated by default and there's no way to activate them.

So I think this is more a bundling problem than a feature problem.

Does that make sense? 🙂

@guybedford
Copy link
Owner

If you control the script loading order, you could ensure that es-module-shims is always loaded immediately before a importShim.polyfillLockdown() call which could be irreversible? Or do you not have that kind of control over the environment?

I'm open to treating this as a distribution problem over a configuration problem, just want to be sure.

@luisherranz
Copy link
Author

We can structure it that way in the code included in WordPress Core, but people can still filter final HTML and remove the importShim.polyfillLockdown() string.

I'm open to treating this as a distribution problem

Nice! Any idea of what could the next steps be? I'll happily work on it if you point me in the right direction 🙂

@guybedford
Copy link
Owner

How about something along the following lines:

  • A new major version of the project, where instead of shim or polyfill modes being detected / configured, there are two separate builds - shim build and polyfill build
  • We remove the existing shim mode enabling or disabling, replacing the shimMode check with a new constant - IS_SHIM.
  • We integrate this constant into the build process just like we do debug builds currently via ESMS_DEBUG (https://github.com/guybedford/es-module-shims/blob/main/rollup.config.js#L33)

Ideally other than that there shouldn't be a lot of code restructuring - it should mostly just work out through dead code elimination between the builds.

We would then end up with the following build dist variations for a 2.0.0 release:

  • esms.polyfill.js
  • esms.polyfill.wasm.js
  • esms.polyfill.debug.js
  • esms.shim.js
  • esms.shim.wasm.js
  • esms.shim.debug.js

How does that sound?

@guybedford
Copy link
Owner

There is also prior art here in #258.

@luisherranz
Copy link
Author

That sounds like a perfect plan, yes.

Since you are willing to make a new major version for this, would it also be a good opportunity to take a look at officially supporting conditional loading of the polyfill as well? Do you have any thoughts on how that would work?

@guybedford
Copy link
Owner

Great to hear. And yes exactly, you're 100% right - once we have a polyfill-only build it can make sense to do the conditional loading optimizations now separate from the feature detection layer. We could tackle both projects together, or do one and then the other. The important thing here will be to ensure that the shim build remains a singular build without any async importing, for that reason I'd expect it might be easier to tackle the polyfill build first then the conditional loading as a separate follow-on PR to achieve this.

Please let me know if there's anything I can do at all to help here further @luisherranz. Always happy to hop on a call to discuss any of the work.

@luisherranz
Copy link
Author

Perfect. Thanks, Guy. I'll take a look at the code and build process as soon as I can and I'll get back to you 🙂

@luisherranz
Copy link
Author

@gziolo correctly realized that if we load the es-module-shims dynamically only when the browser doesn't support import maps we won't need to create a polyfill-only version because WordPress extenders won't be able to rely on the shims because they may or may not be loaded.

That's great news because it means that we can use the current es-module-shims version in WordPress, and the only thing we need to do is loading it conditionally 🙂

So, thank you so much for your willingness to help us with this, but in the end, it won't be necessary. I'm going to close this issue now as resolved.

@gziolo has also been testing an alternative way to load the import map dynamically using document.write instead of document.append, and initially that seems to work fine. We still need to investigate a regression in performance which we haven't figured out yet, but at this moment it seems like we might be able to solve it ourselves.

As I'm going to close this issue because it is related to the polyfill-only bundling, I'll report our findings in the one that is already opened for dynamic loading:

@luisherranz luisherranz closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 2024
@guybedford
Copy link
Owner

@luisherranz thanks for sharing here, this sounds like the right approach to me.

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

No branches or pull requests

2 participants