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

Option to allow overriding existing import map entries in shim mode #254

Closed
lewisl9029 opened this issue Feb 8, 2022 · 6 comments
Closed

Comments

@lewisl9029
Copy link
Contributor

Hi again! 🙂

In an earlier PR we added this check to throw when users attempt to override an existing import map entry.

Fast forward to today, and I actually found a nasty edge case in my custom react-refresh workflow that requires overriding existing import map entries to fix. 😅

Would you be open to adding an option to bypass the check? Maybe in the form of an allowOverrides init option that defaults to false and only takes effect in shim mode?

Happy to send over a PR if you're ok with the idea. Thanks!

@guybedford
Copy link
Owner

Hi @lewisl9029, agreed this could be a useful option to add. If you're able to work on a PR that would be great. Please just try to keep the byte size of the implementation as small as possible!

@lewisl9029
Copy link
Contributor Author

@guybedford random idea related to your comment on byte size: wdty of potentially having a separate build for shim vs polyfill mode to further optimize byte size? It seems unlikely for a site to need to switch between two modes, and even more unlikely that it would ever work.

Might be as straightforward as replacing the shimMode export to a build-time injected boolean and reconfiguring the bundler. Will be a breaking change for users depending on automatic shim-mode detection, but I think it might be worthwhile longer term.

Happy to explore this as a separate follow up PR if that sounds like something worth exploring!

@guybedford
Copy link
Owner

@lewisl9029 if there could be a significant file saving that could be interesting, although I'm not sure it would be such a big saving as most of the code paths are shared. Separating features might well be a byte size reduction approach in future though eg having extensions loaded in based on what is needed. At the same time I quite like that it's one thing at the moment, does simplify the integration process too.

@lewisl9029
Copy link
Contributor Author

Sounds good, I think I'll give it a shot in a quick and dirty way to explore how much we could potentially save, then we could decide if it's worthwhile once we have some actual numbers.

@lewisl9029
Copy link
Contributor Author

PR is ready for review here: #257

Also opened a draft to test out the separate builds idea: #258

TL;DR: you were right, the savings were pretty insignificant (<0.5kb). 😅

@guybedford
Copy link
Owner

Landed in #257.

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