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

Feature: Promised Components #15

Closed
wants to merge 1 commit into from

Conversation

tylerbrostrom
Copy link

Hey @estrattonbailey! Really loving this library. It’s been a perfect "small-but-mighty" solve for a few projects of mine.

This PR effectively solves #7 but without baking in module.default resolution. Pass any Promise that resolves the value of a component call.

Though the API remains unchanged, I might consider this a breaking feature for the fact that mount is asynchronous. Might trip up some folks that call appInstance.emit() (or whatever) immediately after appInstance.mount().

I’ve updated tests and the readme. I thought grouping these readme additions alongside the explainer on the add method might make sense.

Let me know what you think! Cheers!

Feature Summary

Each passed component is now wrapped in a Promise.resolve, which has the following consequences:

  1. You can now pass promise-wrapped components to picoapp and appInstance.add.
  2. Picoapp requires that Promise be polyfilled for browsers that don’t support it.
  3. appInstance.mount() returns a Promise that resolves when all components have been successfully resolved and bound to the DOM.

On picoapp.mount(), each component is now wrapped in a Promise.resolve, which has the following consequences:

1. You can now asynchronously pass components to `picoapp` and `appInstance.add`.
1. Picoapp requires that `Promise` be polyfilled for browsers that don’t support it.
1. `appInstance.mount()` returns a `Promise` that resolves when all components have been successfully resolved and bound to the DOM.
@@ -9,6 +9,8 @@
> [operator](https://github.com/estrattonbailey/operator) – where page
> transitions can make conventional JS patterns cumbersome.

**Note**: You’ll need to polyfill `Promise` for browsers that don’t support it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good shout 👍

@lmartins
Copy link

lmartins commented Dec 5, 2019

@estrattonbailey Eric, any chance we could get an update on this?

@lmartins
Copy link

@estrattonbailey @bro-strummer would be really cool if we could get an update on this, either to say we will be moving in this direction, or not.

@estrattonbailey
Copy link
Collaborator

Hey, I'm sorry y'all. Just moved from NYC to Chicago and haven't been online much. Hoping to get around to some open source over the holidays, starting next week.

@lmartins
Copy link

lmartins commented Jan 13, 2020

@bro-strummer I'm struggling to use your version when using Parcel for code splitting. Parcel gives me a promise, but when I try to give picoapp the resolved promise for some reason the component never gets the node element, just the state. Have you seen something like that?

EDIT: Nevermind, it was a problem resolving the default export from the imported module.

@tylerbrostrom
Copy link
Author

Closing this!

Having revisited the project that inspired this PR with fresh eyes, I now see that this feature—and thus my forked package, picoapp-async—wasn’t strictly necessary for my use case 🤦.

I don’t plan on maintaining the forked package and will eventually be unpublishing it from NPM, so as not to clutter the ecosystem with abandonware.

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

Successfully merging this pull request may close these issues.

3 participants