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

✂️ Cleanup: remove component interface from contextprops #33427

Merged
merged 1 commit into from
Mar 23, 2021

Conversation

dvoytenko
Copy link
Contributor

We've delayed the use of context components to see if all cases can be replaced with Preact. So far we haven't needed them, so it's a good time to clean them up.

Closes #30952.

@dvoytenko dvoytenko marked this pull request as ready for review March 22, 2021 23:06
@samouri samouri assigned samouri and dvoytenko and unassigned samouri Mar 22, 2021
Copy link
Member

@samouri samouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't review closely as it would be pretty difficult to figure out the cleanups given component/test-component were also moved. If we don't need component-hooks in the end thats a win! Although it would have been really interesting to see the whole codebase rewritten with them.

PS. You could try releasing this as a standalone lib called domhooks. Was pretty interesting idea to attach the React hooks/effects concepts to dom nodes.

@dvoytenko
Copy link
Contributor Author

Didn't review closely as it would be pretty difficult to figure out the cleanups given component/test-component were also moved.

For posterity, here's the diff for moved files: https://gist.github.com/dvoytenko/172307cbf8e2bb2db559c8fb2ad218d6

PS. You could try releasing this as a standalone lib called domhooks. Was pretty interesting idea to attach the React hooks/effects concepts to dom nodes.

Definitely something to consider.

@dvoytenko dvoytenko merged commit b30ec85 into ampproject:master Mar 23, 2021
@dvoytenko dvoytenko deleted the run3/clean-context-comp branch March 23, 2021 00:42
@jridgewell
Copy link
Contributor

dist/v0/amp-accordion-1.0.js: Δ -0.47KB
dist/v0/amp-accordion-1.0.mjs: Δ -0.52KB
...

-.5kb from every Bento component! 💣

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

Successfully merging this pull request may close these issues.

Bento: custom-element/Preact binding
4 participants