Skip to content

synced version #1

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

Closed
jantimon opened this issue Apr 24, 2014 · 10 comments · Fixed by #3
Closed

synced version #1

jantimon opened this issue Apr 24, 2014 · 10 comments · Fixed by #3

Comments

@jantimon
Copy link

Hi,

I would like to use your file inside grunt however this would have to be synchronous.
Could you please provide a version which uses fs.readFileSync ?

@pscheit
Copy link
Member

pscheit commented Apr 24, 2014

hey did you know that grunt can be asynchronous?

@jantimon
Copy link
Author

Thanks for your quick response.
Afaik it is only possible to have asynchronous tasks but not to have asynchronous Gruntfiles.
Or is this also possible?

@pscheit
Copy link
Member

pscheit commented Apr 24, 2014

Okay, I thought you meant grunt task.

I would not prefer refactoring it to a synchronous version because the error handling had to be with exceptions then.

Maybe there is a better solution for you with using an asynchronous task to manage the requirejs config. What are you trying to accomplish?

@jantimon
Copy link
Author

My gruntfile contains different tasks which build i18n files, css files, fonts and so on.
Now I would like to limit the source files to files which belong to the requirejs module in use.

So if you don't require moduleX/moduleX.js it would not include moduleX/moduleX.css
By now I am doing this with madge and a short helper script which evals the requirejs config.
However I would prefer dropping the eval and use your library instead

@pscheit
Copy link
Member

pscheit commented Apr 24, 2014

okay I see.
Maybe it helps to put your code from the gruntfile.js into an asynchronous grunt task and use this task to configure and run the other tasks dynamically.

you can use grunt.config.set() and grunt.task.run() for this.

grunt.registerTask('dynamic-task', function() {
  var done = this.async();

  // use requirejs reading here
  configFile.read(function(err, config) {

   // var myDynamicConfig ...

    grunt.config.set('mytask.mytarget', myDynamicConfig);

    grunt.task.run('cucumberjs:run');
    done();
  });
});

or maybe as a last resort there a libraries that convert async calls into sync calls :)

@jantimon
Copy link
Author

jantimon commented May 1, 2014

My idea was not to replace the asynchronous part but to add an additional synced version which uses fs.readSync instead of fs.read

I guess you would have to do some rewrites as you put it all in one big function instead of splitting it up into parts.

After all it was just a feature request - if you are not up to it that's okay.

@mrjoelkemp
Copy link

+1 for the sync api. I'm using this lib within a node tool that has no need to be async. In fact, it would be cleaner and more efficient for a sync api.

@pscheit
Copy link
Member

pscheit commented Jul 22, 2014

okay guys, got me convinced.
give me some time

@pscheit
Copy link
Member

pscheit commented Jul 22, 2014

i admit that changing the code to sync the tests become more clear as well. So thank you for the feedback. Please test it in production and give me feedback.

@mrjoelkemp
Copy link

Looks good! Loving it.

Thanks,
Joel

On Tue, Jul 22, 2014 at 5:49 AM, Philipp Scheit [email protected]
wrote:

i admit that changing the code to sync the tests become more clear as
well. So thank you for the feedback. Please test it in production and give
me feedback.


Reply to this email directly or view it on GitHub
#1 (comment)
.

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

Successfully merging a pull request may close this issue.

3 participants