-
-
Notifications
You must be signed in to change notification settings - Fork 324
Remove Layout.update
as a public API
#419
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
Comments
rmorshea
added a commit
that referenced
this issue
Jul 8, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case.
rmorshea
added a commit
that referenced
this issue
Jul 8, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case.
rmorshea
added a commit
that referenced
this issue
Jul 9, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case.
rmorshea
added a commit
that referenced
this issue
Jul 9, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case. After these refactors, `ComponentTypes` no longer needs a unique `id` attribute. Instead, a unique ID is generated internally which is associated with the `LifeCycleState`, not component instances since they are inherently transient.
rmorshea
added a commit
that referenced
this issue
Jul 9, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case. After these refactors, `ComponentTypes` no longer needs a unique `id` attribute. Instead, a unique ID is generated internally which is associated with the `LifeCycleState`, not component instances since they are inherently transient.
rmorshea
added a commit
that referenced
this issue
Jul 9, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case. After these refactors, `ComponentTypes` no longer needs a unique `id` attribute. Instead, a unique ID is generated internally which is associated with the `LifeCycleState`, not component instances since they are inherently transient.
rmorshea
added a commit
that referenced
this issue
Jul 12, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case. After these refactors, `ComponentTypes` no longer needs a unique `id` attribute. Instead, a unique ID is generated internally which is associated with the `LifeCycleState`, not component instances since they are inherently transient.
rmorshea
added a commit
that referenced
this issue
Jul 12, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case. After these refactors, `ComponentTypes` no longer needs a unique `id` attribute. Instead, a unique ID is generated internally which is associated with the `LifeCycleState`, not component instances since they are inherently transient.
rmorshea
added a commit
that referenced
this issue
Jul 13, 2021
The `Layout` is now a prototype, and `Layout.update` is no longer a public API. This is combined with a much more significant refactor of the underlying rendering logic. The biggest issue that has been resolved relates to the relationship between `LifeCycleHook` and `Layout`. Previously, the `LifeCycleHook` accepted a layout instance in its constructor and called `Layout.update`. Additionally, the `Layout` would manipulate the `LifeCycleHook.component` attribute whenever the component instance changed after a render. The former behavior leads to a non-linear code path that's a touch to follow. The latter behavior is the most egregious design issue since there's absolutely no local indication that the component instance can be swapped out (not even a comment). The new refactor no longer binds component or layout instances to a `LifeCycleHook`. Instead, the hook simply receives an unparametrized callback that can be triggered to schedule a render. While some error logs lose clarity (since we can't say what component caused them). This change precludes a need for the layout to ever mutate the hook. To accomodate this change, the internal representation of the layout's state had to change. Previsouly, a class-based approach was take, where methods of the state-holding classes were meant to handle all use cases. Now we rely much more heavily on very simple (and mostly static) data structures that have purpose built constructor functions that much more narrowly address each use case. After these refactors, `ComponentTypes` no longer needs a unique `id` attribute. Instead, a unique ID is generated internally which is associated with the `LifeCycleState`, not component instances since they are inherently transient.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
Layout.update
method is never used in testing, and users of the abstraction shouldn't ever need to do so either since they ought to be using hooks instead. We should remove this API and find some better way for hooks to trigger a render.The text was updated successfully, but these errors were encountered: