-
Notifications
You must be signed in to change notification settings - Fork 375
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
Cleaning up (undefining) custom elements? #754
Comments
It may. What we'd want to see before committing to this sort of work is memory profiles of popular custom element-using sites, showing that a large portion of their memory usage comes from no-longer-used custom element definitions. Are you interested in collecting such evidence? |
I'm interested, but not sure how to find the sites. They should have many client-side routes that load route-specific JS which includes new elements for the route. At some point my own site will do that. |
In my experience, with one of the heaviest components ecosystems that I know of, the component's registry is never the bottle neck, not even close. On top of that, this seems like a lot of work for so little reward. What if the component itself kicks in some services that will eventually attempt to create instances of itself? Who is in charge? |
Closing, happy to reopen once there's a clearer demonstration this can be useful. |
The fact that custom elements can’t be redefined essentially makes them unusable within Observable’s reactive notebook environment. Our runtime is predicated on being able to re-evaluate cells whenever either the code in a cell changes, or a value it references changes. So, you can’t really use custom elements within Observable, because every time you change the code, you can’t cleanup the old definition before you re-evaluate the cell. |
Redefining custom elements is also useful for:
|
@annevk Please reopen. It's silly to have an irreversible process. |
I think #754 (comment) clearly states what is needed, no? |
@annevk I think that comment is less relevant now, given the later comments. |
Wouldn't most of use cases here be addressed by #716? |
@annevk Look at mbostock and stevenvachon's comments. Our concern isn't about freeing up memory, it's about the ability to replace a custom element after it's already been defined. |
And think realistically about what actually happens on a website. Many websites use vendor Javascript and sometimes vendors do things that collide with the main functionality of the page. In enterprises, a developer may not have a choice to use a different vendor as it is decided by the business. Having the ability to work around misguided code is invaluable for Javascript. |
@rniwa Scoped custom elements are a partial solution, but still not when testing the default registry. |
What you'd do in that situation is to override what |
@rniwa then my custom element would be atonomous. |
What would happen to an object representing a custom element instance which has become ‘undefined’? It presumably has the prototype associated with the custom element and likely has local state via own properties, private fields, or WM privacy. These things can’t be removed after the fact (the object may not even be extensible). Even if the object representing the node could be replaced somehow in the DOM, references to the original object may exist in closures. This doesn’t seem coherent to me. |
In addition to defining & collecting compelling use cases, here are some of additional issues / edge cases that need to be sorted out off the top of my head:
I'm sure there is a lot more cases to consider that I haven't even thought about. |
How about not changing anything for these existing instances, in the same way that if you call |
I think you’re on the right track that it would need to behave something like that @mbostock. I.E. what’s being disassociated in the registry is ‘this tag name points at this definition’, like setting the name and local name parts of the definition to null. I think that would imply in turn that I think this would create the possible scenario of multiple elements with the same tag name occurring with two or more non-unknown meanings in the DOM. I don’t think that can currently happen with autonomous elements and am unsure what implications it might have. |
That's a given; there totally needs to be behavior like that. 👍 This downgrade process may be complicated, but ability to clean up memory usage is a critical part of designing any API that creates memory usage. Even if we (any one of us) haven't seen a use case where memory growth from defining new elements is a specific problem, just the act of designing any re-usable and repetitive API with irreversible memory growth is simply bad practice that should be avoided. "Downgrading" elements has intricacies to think about, but it's probably something important to think about for the longevity of web-tech beyond simple short-running applications. Hypothetical Scenario 1, Users with many windows and tabs:Imagine everyone drops React/Angular/Vue for plain Web Components. Now imagine a user that has 30 Chrome windows open, with let's say an average of 10 tabs per window (I've seen worse than this in practice). Now imagine that after loading custom elements in many tabs, and switching routes (etc), on average each web app in every tab uses 300 custom elements over the app life time, and on average only needs 20 custom elements at any moment. Without disposing any custom elements, and with current state of APIs (no cleanup of definitions), this means we'll load 90,000 Custom Element definitions. Now, if there was a way to clean things up, this number would be reduced to 6,000 loaded at any point. That's a lot better, and could improve user experience when switching tabs often, especially on mobile devices. Hypothetical Scenario 2: Web-based OSA web-based OS-like system that loads "micro apps" by loading Custom Elements in a shared DOM. Over time, memory growth from using (installing) many apps will grow and never shrink. Hypothetical Scenario 3: low-power low-cost embedded devicesWe need to squeeze things into small amounts of memory. Easy to imagine problems here... Hypothetical Scenario 4: Long running gameLike Half-life, this game might be an infinite world game, and we'd need to unload the components as we leave various areas of the game. All the cool kids like efficient game engines and micro-optimizations that eventually add up. Yeah, let's make FPS's with HTML! Hypothetical Scenario 4.5: "Infinite-world" gameSimilar to the previous idea, in an infinite world game where users can travel to many places and cover large amounts distances with changing scenery, landscapes, characters, and items, you can imagine how leaking every definition of every scene, landscape, character, and item could be a problem. Yeah, let's make infinite-world games with HTML! Hypothetical Scenario 5: TestingIt becomes is simpler to write tests when there's no naming conflicts. Hypothetical Scenario 6: Live code editorsReplacing elements with new implementation on code changes. If we do something like add an incrementing number to the end of an element name, then the user CSS won't work unless you painstakingly write a system for that too, because selectors with tag names will break. And then it won't be "just CSS" anymore. Hypothetical Scenario 7: Live code pushes/patches or HMRImagine an in-app purchase system upgrades some objects with 3rd-party extensions from an extension store. The objects reload (maybe they are covered with a loading icon for aesthetics), and now the objects have the new features. |
Another use-case is with using micro-frontend architecture. We have a custom-elements library for visual components that is reused through many SPA. The library is integrated at build-time in the SPA, so each SPA can use a different version of the components. At runtime, the first spa that will load the custom-elements will define them and after that every other spa will use the custom-elements first registered even if they are incompatible. In this context, I would like to be able to unregister all custom-elements when the micro-frontend switch from one SPA to another. It would be useful for :
|
@ptrot001 I think Scoped CustomElementRegistry is going to be a better solution there. Once a registry and all associated shadow roots are unreachable, a browser should be able to clean up definitions mostly non-observably (modulo any potential restrictions on classes being definable in at most one registry at a time). |
Came across this while looking for a way to resolve the error:
In an SPA components get loaded and unloaded all the time. It is very surprising that this |
I think the key is to define how scoped custom elements registry would work. Since we'd have to solve many of the issues raised for unregistering custom elements for scoped custom elements registry, I'd imagine we'd be able to tackle this issue better once that's done. |
Meanwhile, please treat the |
It's not that simple. We need an answer to all the questions I raised and probably a lot more questions as we define that behavior. |
Can we re-open this issue? In my humble opinion, any API without cleanup methods should always have its cleanup issue remain open, regardless of how long it takes to find a solution, because it is simply best practice to always have cleanup in mind when making any API. Having this issue closed discourages a good mindset that, in my opinion, every developer should have.
Two ideas:
I think option 2 is a lot better. However I think an idea similar to
If we go with option 2 above, this is easier. They simply still have the same reference, and the referenced element may have cleaned itself up. Going with option 1 would be more complicated, and it would lead to the same problems people had before web components and before mutation observer: how would one clean up when an element is removed (or downgraded)?
Another DX improvement (in my opinion) would be to add lifecycle callbacks to
If the element undefines itself (for whatever reason),
This would work the same (or with little difference) regardless of options 1 and 2 above. But still i think this makes more sense with option 2.
Got any more? The above seems fairly straight forward: downgrading is just a simple object manipulation just like upgrading is (going with option 2, which seems better):
Wdyt? |
As I have previously stated, there are a number of questions that need to be answered for this to work that will be resolved once the scoped custom element registry is well defined. So folks who are interested in this cleanup capability should also focus on the scoped custom element registry since having that feature not only addresses most of important use cases raised in this issue, having a concrete of scoped registry would pave a way to define how this cleanup feature will work. |
I don't think scoping resolves cleaning the use case I mentioned above, as Section A can be removed from page then later added again. Presumably the scope would have been the same unless the page itself implemented some scope naming rules. Otherwise, there will still be an error redeclaring the same component. RE your edge cases, I'd propose something like All this though is something I think we should only work on after the issue is reopened, and @annevk wants us to demonstrate use case first. |
@imdavidmin @getflourish if you're looking for hot module reload, testing, etc, you can still do all that in user-land, here is a POC of that for you to test: |
Respectfully, it is not difficult to work something out in Javascript or with a library. The issue at hand here is what I believe to be a design oversight in Web standard specs. I already am working around the issue but just not very elegant. |
It wasn't really an oversight. We specifically designed so that custom element can't be redefined since allowing re-definition raises all sorts of design questions we weren't comfortable answering at the time. If you're interested in this issue getting any traction, as I have stated numerous times earlier in the thread, please follow up with the discussion of scoped custom element registry. |
By no means I'm saying the current spec is done haphazardly, I'm sure a lot more deliberation than myself thinking have gone into it. Regarding scoped registry, I've already answered why I think that doesn't resolve the problem completely, in the case of re-loading the same contents. |
Sigh... this is the third time I'm having to explain this. Adding the capability to re-define custom elements or delete the definition, we need to answer some questions I had previously stated. As I have previously stated, figuring out how scoped custom element registry would answer most of those questions. So, if we have figured out how to implement scoped custom element registry in the browsers, it would pave a way to add the ability to clean up custom element definitions in the global scope as well. So the step 1 of this feature is to work on the scoped custom element registry. |
@rniwa I see there is a tentative answer at #754 (comment) with no feedback. I just want to understand the next steps to work here. Thanks! |
Part of the trouble here is that JS itself has no generic concept of "object teardown" to build off of, so there can be no answer involving "transitioning" the original object that is not at best idiosyncratic and at worst incompatible with core language features like private fields (which would throw on attempted reallocation if the definition were added back). |
All the use-cases stated on this thread are targeting development time. I'm very sympathetic with that, and I have seen this first hand, that's why I wrote that little library, but it is my opinion that such feature can wait, you have workarounds, and we are in the process of getting scoped registries. Once we gather a lot more info, and see how developers -- specially big libraries authors -- respond to scoped registries, we can revisit this decision, to see if this is truly needed. |
Here's another dev-related use case:
yes, this is 'just' a development-related concern. however, as I like to say, "DX is UX". The experience of your developers just is a user experience, as developers are also users. And DX, like any other UX, strongly determines adoption. All I need to be happy here is the ability to mutate an entry in the registry -- no 'unloading' logic, just good ol' idempotence. But because I can't get it, I think I'm stuck with CSS classes. Web components, we had a good 7 minute run |
@elmarsto just use a tool to allow you to do HMR during development, that's the basic workaround that I mentioned above, here is one: https://github.com/caridy/redefine-custom-elements |
@rniwa ,
The software is soft enough that even craziest things could be done. Just a question of effort. If the polyfiull with good enough performance can be done, it means in native browser it would be done even more efficient. It is not an argument that "difficult to implement" to ban the standard proposal. The proposal can be put on hold with query to provide PR to one of OSS browsers The unregistering custom element is a part of facade pattern which is quite useful in AOP approach of incremental features associations on the component. Those are (but not limited to)
@annevk , would this argument be sufficient to reopen proposal? Note none of the samples are specific to development environment. Those needed in prod. Of course there are work around. And even unregistering can be polyfilled as of now. The question is to keep debate on why needed and how to implement better. Not how to dump brilliant ideas. |
@sashafirsov most of the time it is not really about whether we can do it or not, or whether it can be polyfilled or not... most of the time it is about the unknowns, and whether or not we will put ourself is a corner if we implement a particular feature with respect to future optimizations, or whether or not we will probably hinder on some other more prominent future idea. For this particular case, as a creator of one of the polyfills (https://github.com/caridy/redefine-custom-elements), and as a participant on many of the discussions about this on the various groups with implementers, I just don't think we should implement this ATM in the UA, we can wait and see. |
@caridy , @annevk, could you, please, elaborate on the criteria of reopening of proposal? Is it contest of polyfill popularity, upvotes on the issue, or something else? I see the chicken and egg problem. Without proper proposal, even polyfill would not be created. Without implementation or polyfill there is no way to track the uses or potential use cases. Also the API on any kind of object in browser should be functionally complete: create-attach/request-list/watch/remove-replace. Perhaps I am missing some more on the generic entity life cycle. If the API have been created with one leg, it shall be treated as insufficient. |
Here is a demonstration of why it would be useful: There are countless other demonstrations of the usefulness of undefining custom elements which have been posted by others in this ticket. @annevk please re-open this ticket. If there is anything else that you feel is needed in order to re-open the ticket then please let us know specifically what that is and in what form it's required. Or perhaps re-open it anyway so that what is required can be openly discussed. |
use case where this is helpful: if i download two dependencies that both try to define the same custom element (possibly because they each separately invoke that dependency), and those dependencies don't check if the element is already defined, i could use "undefine" to prevent the second one from failing. the current state of play is that i can do nothing about this. |
I wouldn't say there's nothing you can do. Not that it is exactly desirable to do this, but:
|
This whole issue is so infuriating it is pretty much pushing me away from custom elements and back to React. There are reasons to undefine an element, definitely for hot module reload which I am trying to implement with lit and vite. Some of the commenters on here 🤡 |
This comment was marked as spam.
This comment was marked as spam.
Libraries should not be automatically registering their custom elements in this way without providing a non-automatic route too. While it can be useful to just include the script in a plug and play manner, it is too rigid and, if you want to extend or customise the element before registration, the registered element may never even be used. I'd also find it strange for a library to register custom elements from its dependencies or allow its dependencies to register custom elements without any attempt at scoping or isolation, that's already a recipe for problems. It's akin to using a fixed global variable to store information or defining a global function as the main entrypoint for your library (a problem that modules were meant to solve). I try to ensure the output for my custom element libraries is either the custom element class, which the page author can register themselves, or a function that will register my custom elements and any dependency custom elements under a provided tag name/prefix. Even if the author has not provided an official approach to importing the element without registration, you might be able to import it from the dependency source code directly: // Automatically defines <awesome-widget>
// import AwesomeWidgetElement from 'awesome-widget';
import AwesomeWidgetElement from 'awesome-widget/src/AwesomeWidgetElement.js';
customElements.define('my-awesome-widget', AwesomeWidgetElement); |
This worked perfectly for me |
I notice many web-based libs/APIs focus on creation without reciprocal destruction mechanisms. They assume that cleanup happens when the "web page" is closed.
A long running, wanting-to-be-robust, web application might like to unload elements if it knows they are not being used on the page any more (f.e. it switched to another client-side route). It may like to lose references to classes, and can fetch the relevant JS (cached) later.
Would it make sense to be able to unregister elements, to free up memory?
The text was updated successfully, but these errors were encountered: