Component loading interface - replacing isTrusted
#71
Replies: 4 comments 5 replies
-
This is a really interesting point and I hadn't considered some of the situations it presents. What is desired behavior if you embed a component as trusted but it has a child which is not marked as trusted? My initial instinct is that any trust setting applies directly to that component but is not automatically propagated to its children. Say author A embeds a component from author B which in turn embeds a component by author C. I think it would be desirable to give the ability for author A to embed author B's component in a way that denotes that author B's code + decisions are trusted, with one of those decisions being to sandbox the component by author C. This would mean sandboxing is default at every step in the tree which is not explicitly marked trusted.
That makes sense, I can see use cases for more advanced options. Presets could be nice at least for the most common usage, but I would suggest just switching to the verbose version for our development then we can later circle back to adding shorthand as we learn about how trust modes are frequently utilized |
Beta Was this translation helpful? Give feedback.
-
I'll open an issue to address the current loading implementation separately. What's there now is actually slightly inverted - when A loads B as trusted, B's definition should be inlined into A and rendered in A's container. But currently B gets its own container and all its descendants are rendered within it. This isn't a fundamental change, the traversal just needs to be reworked. |
Beta Was this translation helpful? Give feedback.
-
I agree with this thought process @mpeterdev. This is how I'd imagine things working. I do like the idea of having the ability to globally/recursively trust components published by yourself or a specific account. |
Beta Was this translation helpful? Give feedback.
-
I've addressed the issues with trusted loading; needs some testing but should be most of the way there. The big caveat here is that it requires Here's a demo Component that helps visualize what the Component tree looks like when each Component renders one trusted child and one untrusted child - i.e. there are 2 roots (1 trusted, 1 sandboxed), 4 parents (1 trusted under trusted parent, 1 sandboxed under trusted parent, 1 sandboxed under trusted parent...) etc. for 5 generations. Once that's sorted I'll update the interface. I'm thinking the prop could just be called <Widget trust={{ mode: "trusted" }} ... /> I'm very open to suggestions before finalizing, and will post the PR here when it's ready. |
Beta Was this translation helpful? Give feedback.
-
The current Component loading implementation only makes a distinction between
trusted
andsandboxed
via theisTrusted
flag. While convenient during prototyping, this seems like something that should be addressed now to avoid the first set of Components using an inflexible API that will invariably change.In the current implementation, a Component tree is parsed from the given root Component. Any Components specifying
isTrusted={true}
will have their entire descendant tree rendered within one container. This happens even if a descendant explicitly specifiesisTrusted={false}
, but this is an unimplemented feature and not necessarily the functionality we want.However this interface unnecessarily obscures what's actually happening and puts constraints on developers trying to implement secure loading in their Components. If we switch to an interface requiring an enum value for "loading presets", we can elucidate the current implementation and incrementally extend the API to implement loading strategies, e.g. loading Components from the same dev as
trusted
or using a whitelist.In the short term, this would mean going from:
to:
However in the long term, extending this API with parameters for these loading presets makes it more powerful for devs. This would benefit from a more configurable implementation, e.g.:
So if we think that's something we might want someday, we can implement just the verbose form now and not bother with the string values:
to:
I'm leaning towards the explicit approach up front. Defining string presets for devs is appealing, but allowing for either a string or object complicates the interface to save some keystrokes. Though maybe presets are fine as long as they're strictly shorthand for a more verbose configuration, i.e. these would be equivalent:
Any thoughts welcome, especially any on parameter names 🙂
Beta Was this translation helpful? Give feedback.
All reactions