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 a new onReadFileError optimizer hook #697

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gfranko
Copy link

@gfranko gfranko commented Jun 18, 2014

What: A new onReadFileError optimizer hook that is called whenever the Require.js optimizer cannot find a file dependency.

Why: This will allow developers to add custom support for fallback file lookup algorithms (e.g. node.js)

Example:

onReadFileError: function (moduleName, path) {
  var fs = require('fs');
  // If a file is not found with the Require.js configurations,
  // use the node.js file lookup algorithm
  return fs.readFileSync(require.resolve(moduleName), 'utf8');
}

Questions: I could not get the Require.js tests to complete successfully with or without this change. Could you take a look and let me know what else I need to do to integrate this PR?

Thanks!

@jrburke
Copy link
Member

jrburke commented Jun 30, 2014

Thanks for looking into this, very sorry for the delay -- with the summertime, work is a bit more busy as we need to cover for each other while some of us go on vacations, and I have been taking some time off too.

I'm open to figuring out how to get this to work. On the tests there are some assumptions on file layout:

https://github.com/jrburke/r.js#directory-prerequisites

then going into build/tests and running alln.sh will run the main optimizer tests in node.

On the code change, on first thought, I think the file.readFile* methods are too low level to put this change, since those methods, readFile in particular, are used in code that does not have module ID or context as part of the caller.

What might be needed is a way for code to intercept context.nameToUrl calls instead. By allowing those interceptions, that will match well for the future too, as that relates better to the ES6 locate loader hook.

What about allowing the config to have a onNewContext: function(context) {} call, and then for this particular example, it would override the nameToUrl. The override would call the original nameToUrl, then do an path.existsSync() on it, and if not exist, do the node fallback.

If that sounds good, then I think the modification would be done in requirePatch.js, at the end of its newContext override

What do you think?

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