-
Notifications
You must be signed in to change notification settings - Fork 11
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
modules that require globs cannot run on native nodejs (without applying the transform) #11
Comments
I am inclined to say that this goes beyond the scope of this plugin... However you are right that globbed requires could be a very useful feature in server-side code as well. EDIT: Actually, a similar module already exists that does this: require-glob |
I agree that it is important to keep the require call as close to the original as possible. This is why I think it should be left alone (that is the closest possible) and glob-requiring functionality should be moved to a function of a different name. There is an inherent value in conforming to the commonjs (or common-j-esque as it is implemented on node) module standard. However, modules depending on this transform currently conform to neither. This is why for requiring text files, substack (creator of browserify) also created the fs.readFileSync transform, rather than allowing us to require('some.html'). I cannot find the reference for this, but i recall reading his explanation saying that he did not want to change the "require" interface. This makes sense, since browserify was built to work with node modules, not some new kind of module format. In practical terms, my objective (which would be useful to me and probably many others) is to be able to require files via globbing expressions and have it work on BOTH node and the browser. This project lets me do this for the browser, and "require-glob" lets me do this for node, but currently there is no complete solution for said objective. If you would consider changing this transform to find & replace "requireglob" calls instead of "require" calls, I would be willing to make the "requireglob" package that provides a server-side runtime implementing the same interface as your current "require". And bingo! Anyone can require globs of files without even caring whether the code will be run on node, the browser, or both. |
I've thought long and hard about this, and I'm not entirely sure I completely agree with your reasoning. This would really be trying to do two different things in the same package:
The difference with your provided example being that fs.readFile[Sync] is already an established and well-known API, whereas requireGlob would be a browserify fix for an API that doesn't even exist yet. Since we're already "browserifying" something that doesn't exist, I figured we might as well fake the API we would want from require(). |
The problem is that (given that we want maximum portability) we want to use the existing require() syntax. We can easily fake a new requireglob() function rather than override the very well-defined and important existing require() function. I think the complete solution would require those 2 things, but not in the same package. There would be the transform, "require-globify", and then the runtime package, "requireglob", which would be used on node but stripped out by the transform. |
Fair enough. I'm going to start work on this in a couple of hours. There is one thing that still (slightly) bothers me though. I really believe globbed requires is something that should be baked into the require functionality. That is why the transform highjacks the official require() call. It adds a bit of authority/legitimacy to the functionality. |
Exciting! :D Does that mean this repo/package/module/transform is renamed to require2ify? The other thing I think require() is missing is the ability to require different file types (other than js and .json) as a String or Buffer. I am working on this now. Maybe I should put this in a separate issue, but its probably something im doing wrong. Im trying to provide my own mode plugin function but its not happenin... window.html = require('../../../shared/Site/pages/*.js', {
mode: function(base, files, config){
console.log('mode plugin', base, files, config);
return 'require("lodash");';
}
}); gives me
Help? |
I suppose the name of this package could/would be changed to require2ify or something similar, but we'll see about that once require2 (or whatever the name will be if we can't get that name) is published. |
The donated "require2" npm package name. |
Hot diggity! |
I have some more ideas to share ... |
An update on this: I finally got around to get some more work done on this. I started a git repo and am currently in the process of drafting the spec of how require2 could/should work. You @zenflow or anyone else is very welcome to make suggestions or edits before I continue work on an implementation. |
require2 package on npm is available. Might want to publish your repo even in this early phase, to avoid possibly having to rename the whole package to 'require3' later on. Did you give any thought to starting a 'require2' organisation here on gh? https://github.com/require2 is available I'd really love to have some brainstorming sessions with you (and hopefully others) but I feel like any github issue is the wrong place to get into it. (I really wish there was some messaging on github) I've taken the liberty of setting up a require2 slack team which could help us collaborate. It supports offline messaging (among other neat features) |
You are very right. All further discussion should be continued there or on the slack domain. Speaking of which, @zenflow could you send me an invite to that at adriaan.callaerts@gmail.com? |
Anyone wanting to join us on slack can invite themselves now. @call-a3 want to try it and see if it works? Also added the slackin badge and link to README.md |
ping @call-a3 I wrote you a letter on require2.slack.com |
Read it, will respond in a few hours... |
Proposed solution: rename the 'require' function to 'requireglob' and publish a server-side runtime of that name on npm.
I would be willing to help with the implementation of this.
Thoughts anyone?
The text was updated successfully, but these errors were encountered: