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

Add support for declaring 'contents' as a promise (or a function returning a promise) #11

Closed
wants to merge 4 commits into from

Conversation

papandreou
Copy link

@papandreou papandreou commented May 19, 2017

Just something I hacked up as part of another experiment -- contributing/sharing it here in case it might be useful.

@rmarscher
Copy link
Owner

Thanks for the pull request @papandreou. The ability to pull the contents async from a promise sounds useful. If the promise took a long time to resolve, I think there could be a race condition where the webpack compilation starts before the resolve plugin has been attached to the compiler. But I think those resolve plugins might work via async so inside the resolverPlugin, it could detect if the contents are a promise and resolve it before populating the virtual filesystem and calling the plugin callback.

This pull also seems to be causing one of the tests to fail. I'll ping you when I've pushed something up that does what I outlined above and maybe you can test if it works with your experiment. Might be a little bit before I get to it though.

@papandreou
Copy link
Author

Oh, I didn't realize there was a test suite -- sorry! I fixed the bug and force pushed now :)

@papandreou
Copy link
Author

Sorry, apparently I attempted to push to the wrong remote repo, which failed without me noticing. Redid the fix and pushed now.

…lugin, which can already be async

For now this does away with populateFilesystem, making the unit tests fail.
…on in a promise

Doesn't work -- maybe this just isn't possible.
@papandreou
Copy link
Author

Played around with it a bit more, but I don't think it'll work. As far as I can tell, it's not enough to have the contents injected by the time the resolver plugin calls its callback. That's probably what you tried to point out in your initial feedback.

@papandreou
Copy link
Author

Let's close this PR until new insight comes up.

@papandreou papandreou closed this May 23, 2017
@rmarscher
Copy link
Owner

@papandreou Thanks for digging into it more. I'll let you know if I find a way it could work.

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.

2 participants