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

Use of promises for readAndPreprocessFileContent, to allow asynchronous operations in scriptprocessor #158

Conversation

kvaggelakos
Copy link

Needed to have asynchronous support in the script preprocessor. Added a callback parameter to the process method and updated the core to use promises internally to accomplish this.

@jeffmo
Copy link
Contributor

jeffmo commented Oct 24, 2014

How does this work for CommonJS, where file reads/pre-processes have to be synchronous to work correctly?

@kvaggelakos
Copy link
Author

@jeffmo The code is not stable for a merge at this point. I am working on solving some issues. Not sure I understand your question, why would they need to be synchronous? Each step of the process (including preprocessing) should happen serially. That shouldn't affect the preprocessing though, we just need to rewrite the serial steps to wait for asynchronous operations (especially in the preprocessor) since we are exposing that to the developer. The reason I'd like to see this personally is because browserify transforms work with streams. Hopefully I'll get this working soon and I'll attach an example to this PR.

@kvaggelakos
Copy link
Author

Digging more into this it seems like what I thought would be minor issues will become re-architecting jest. Even though that sounds like fun, I don't have the time right now unfortunately. Therefore I am closing this PR and adding a feature request.

@kvaggelakos kvaggelakos deleted the async-read-and-process-file-content branch October 27, 2014 13:47
@jeffmo
Copy link
Contributor

jeffmo commented Oct 27, 2014

The high-level goal here is indeed a tricky one -- but one fundamental constraint we have is that node's implementation of CommonJS forces things like reading from the fs (and pre-processing) to be done synchronously. At some point I'd like for us to support other kinds of module systems than just CommonJS such that we can do asynchronous require()s as you're aiming for here.

Additionally one could imagine a more ES6-style async loading pipeline where jest simply blocks the top-level execution of the test script until after all of the recursive dependencies have been (asynchronously) loaded. The only requirement there is that we have a fully statically analyzeable way of expressing dependencies in the module system -- of which node's implementation of require() is not [necessarily]. However, if the module system were pluggable, it would be possible to build a module system plugin that simply punts on this generality and requires that you always pass in string literals to require() rather than variables.

In any case, I really appreciate the thought behind this!

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants