module: fix for .foo.bar style multiple extensions #5376
Conversation
Fixes behaviour where multiple extensions would be picked up by tryFile but not passed properly to require.extensions, leading to strange parse errors if multiple extensions are used.
|
Sorry, the Module system is locked. Changes in this area are too risky. Even if it wasn't, |
|
In this case, it's not so much a "change" as a "bugfix". At the moment, Node can properly handle The immediate use case is for folks who want to write literate-style scripts, where the code is both valid Markdown and valid JavaScript (after a little bit of pre-processing). If Node allows If there's some way to "specify the filename all the way" that I'm missing here, let me know. The use case is for the simple: |
That's what tests are for, it's not a huge change. If this isn't fixed before 1.0 it never will. |
|
I'm somewhat sympathetic to this PR but if there is the slightest chance that it breaks existing code, then I agree with Isaac that it needs to be rejected. The thing that it fixes is not important enough for that. |
|
Fortunately, there isn't even the slightest chance ;) If you examine the code for a minute, you'll see that it's simply looking for each compound extension, from longest to shortest, as a key in Since the final extension is always still checked for, and no one has ever been able to register multi-part extensions in the lookup object before this point, it can't change the behavior of anyones code. To wander a bit off topic, "if there is the slightest chance that it breaks existing code" doesn't feel like a real argument. If you actually mean that, then there probably needs to be better test cases in Node surrounding the Here are some actual reasons for closing this ticket without fixing it:
And please don't take this comment in ill-humor -- I'm not trying to be combative, but I am trying to be persuasive. I'm looking forward to a day when folks can write beautifully annotated Node libraries, and painlessly view them as rich documents and run them directly with Node. |
@jashkenas: Out of interest, are you arguing here for publishing node modules as something else than Javascript? I write practically all my code in CoffeeScript, but I really like the simplicity for the ecosystem to only deal with javascript (with sourcemaps, if needed). Any beauty and/or increased readability made possible by the alt-js language can be easily found in the module's When developing (private) apps however, there's convenience to not having to compile the code manually before running it. |
No, I'm not. I agree with your comment. |
|
I'll say it again: If I had a time machine, or could do so without breaking a bunch of extant JavaScript programs, I'd do away with require.extensions entirely. Relying on Node auto-extension-ing your files is an antipattern that I find disagreeable and destroys more value than it creates. So, no, we're not going to make it more powerful to use this anti-feature. Don't use require.extensions. Ever, for anything. It's terrible and obnoxious. Giving Node the power to load something other than Node modules was a giant mistake that I will regret forever. Compile your code to JavaScript if it's not already. Load it using the built-in JavaScript loader. Specify the filename fully. |
|
I fail to see how the best course of action is to keep a broken feature in core untouched, or why Specifying the filename fully makes code not as portable. Right now we can move between Using require extensions, instead of pre-compiling to .js, also makes it easier to not commit generated js files to a project without adding specific rules to a project's |
They're all simple features on their own. What real-world need does it cater to?
Or, even better, you can specify .js in your code, and compile your compile-to-js to js. Node is a JavaScript platform, built on a JavaScript interpreter. I don't think it's unreasonable that you give it JavaScript, or that you tell it the path to the file that you want it to run. If you must run the coffee-script files directly, run them with the I don't see any reason why require.extensions should exist, except for backwards compatibility. |
|
Just go ahead and remove Leaving it half-implemented leads to more headaches for folks who either try to use it, or get hassled by folks that want to use it. Fixing it or killing it are both paths to a More Perfect Node™ -- leaving it broken isn't really. |
|
100% agreed with @jashkenas. Either remove it or fix it. It would really be an unwise decision to leave it as is. 👍 edit: rephrasing |
|
@jashkenas Would you accept a patch to make coffee-script not use this any more? |
|
Yep. If you're not willing to support it in Node, I'll certainly be happy to remove it from CoffeeScript ... as soon as the deprecation/removal patch lands on Node master. |
|
@jashkenas 7bd8a5a and isaacs/coffee-script@8302a15 (Note: lots of "Unexpected token: INVALID" on the cofeescript tests with isaacs/coffee-script@8302a15) |
|
With the caveat that none of the things that you assert in your commit message are in the slightest way true ... especially now that source maps are up and running properly -- thanks for the patch. I'll merge a version of it with the tests running under older-style loading. |
Yes. On the other hand you are clearly not a coffee-script user, so that is just an opinion. Many things in node core are not strictly necessary, but there for convenience, isn't that the point of a standard library? Disregard this example. I might be misguided, but removing support for require.extensions in coffee-script means it will be forced to either monkey-patch CoffeeScript is the 6th most popular module in node.js, with ~300.000 downloads per month. It's more popular than connect, socket.io, wildly more than jshint. And it makes me money. That's real enough for me.
Care to elaborate? I think it's a complete success, if not for bugs like this. Javascript is becoming a compile target, even Mozilla and Brendan have already acknowledged and encouraged it. |
|
@ricardobeat I'm not complaining about compile-to-js as such. I'm complaining about the unnecessary complexity added to Node's module system in order to compile coffeescript to JS just in time at require() time. Brendan Eich's opinions on the matter have nothing to do with this case. He's not a Node contributor, he's the CTO of a browser company. Please refrain from appeals to authority. Compile your CoffeeScript to JavaScript, or run your coffee programs with the coffee interpreter. The require.extensions approach is unnecessarily convoluted, and not any more portable. It encourages depending directly on the coffee-script module, which 1298 modules in npm do today. Most of those are not in any way related to coffee-script directly, but are merely written in the CoffeeScript language, and doing generic node stuff. Many of them unnecessarily rely on not only downloading an extra dependency, but also running a brittle install script when they could instead use a deterministic approach, compiling their CoffeeScript ahead of time, and publishing JavaScript to npm. Many others rely on require.extensions, which is a shared global mutation that cannot be reliably un-done. (What happens if two coffee-script modules rely on different versions of coffee-script?) I'd like to repeat: I'm not complaining about CoffeeScript, or any other compile-to-js module. I'm complaining about the unnecessary complication added to Node programs by having require.extensions, by using extension-less filenames in require(), and using require.extensions to do just-in-time compilation to javascript.
Sure. Except for all the failures, bugs, unnecessarily complexity, tight-coupling, global mutation, and brittleness, it's a resounding success. If you compile your CoffeeScript ahead of time, there are zero problems. If there is a require.extensions hook, no one will do that. That is why I say it was a huge mistake to add that feature. And the same commitment to stability that keeps me from removing it also keeps me from making changes like the one suggested here. |
@jashkenas I don't see how source maps are relevant. My complaint is about the brittleness of JIT compilation via require.extensions hooks, not with compile-to-js as such. |
This is definitely a big problem. ALL npm modules (for node) should be in npm as js files.
but that means you have to precompile everything to simply run it |
|
Coffeescripts should be compiled to JavaScript in npm |
|
Well i use the require.extensions for loading private modules where people paid for. |
|
In node, "Deprecated" means "Please don't use this. It might have bugs, and we won't fix them. It was a bad idea, and you should find other options if at all possible. It MAY disappear or break in the future." See: https://github.com/joyent/node/wiki/deprecation Deprecation is not a promise that it WILL be removed, merely a statement that we really really think you shouldn't be relying on it, and that it is officially unsupported. Since |
|
Well i think its bad to not support it. And deprecation says it CAN be removed. So i can not build on it and be sure it works on every version the same way. |
@geNAZt More importantly, judging by the response to this bug, you cannot be sure that bugs you find in it will ever be fixed! |
|
Well i can be sure if you set the require.extensions thing to WONTFIX, that it acts the same way in every version. If you say one point extensions are enough then it is okay. But to turn off this feature and say "compile" anything to javascript to use it in node is not the best solution it think. |
|
@isaacs that wasn't an appeal to authority, just goes to show the direction that JS is heading. This freeze is effectively shutting out innovation from the ecosystem, like supporting "Global mutations" is a red-herring; nobody will ever need two versions of coffee-script in a single project, just as they won't need two nodejs versions. Publishing a module depending on coffee-script is in the same league as committing node_modules to your repo, or including huge archives in a npm package. Computers are completely capable of compiling to javascript on-demand, without any hit on application performance, and I prefer not to have machine-generated code hitting the disk. Anyway, this discussion is clearly going nowhere and boils down to personal preference. Let's see where we're at in a year or two. |
Actually, that example was taken from a real life bug. I've had long frustrating discussions with actual humans about why I'm not going to even entertain the possibility of making require.extensions a prototype chain that inherits from the parent module so that you can have two different |
|
I am trying to find a good solution to this, not unsolvable - I think, problem. Have you ever given a proper extension system a thought? If the supported modules were referred in the package.json then multiple sub-modules could have their language support: "fileSupport": { It would allow people to enjoy their syntax of choice for any files and it wouldn't break any logic. The coffee-script project could then extend their install process to something like: sudo npm install cpm -g cpm init . // analog to npm init, with the extended fileSupport parameters If coffee-script would be deployed using this strategy, others will join too and overtime the global .extensions hook will become really obsolete. (as people have a common way to deal with file formats) Also this would make your logic cleaner/easier and encourage the use of npm/package.json even more. Wishful thinking? |
|
@isaacs: it seems to me that there are two distinct issues here, which is leading to some confusion. In terms of your position as reigning node and npm monarch, you have the right to take strong stances on node-as-an-ecosystem, including things like JIT compile-to-js in npm and whether certain patterns of use should be encouraged or discouraged. I think people here largely agree with you on that front and respect the work you're doing to make node a platform where it's easier to write good code than bad code. However, that can't supersede our right as developers to use that platform as we like. I hope you'll agree that there is something deeply unsettling about being told what workflow you're allowed to use by someone without the context to make that decision properly. It feels eerily similar to that "Steve didn't think anyone would want to do this so there's no way to do it" feeling I get with my Apple products sometimes. Ultimately, if I want to JIT compile in my own projects, for my own reasons, ain't nobody going to stop me because it's my damn machine running my damn code. So here's what I propose:
The consequence for you would be that you can now disavow all knowledge of altjs and require.extensions. For people like myself who like being able to require non-js files directly, we can continue to do so where it makes sense for us, up to the point where we want our code to be part of the js-only node ecosystem. We can fix (or even improve!) require.extensions, and probably do other more exciting things besides. We get to keep our workflow, you get a victory in your quest to build a more perfect node. Everyone wins. |
|
@sgentle You're neglecting the dramatic difference in cost, as well as my personal commitment to backwards compatibility. The cost of leaving things as they are is close to zero. If you want to build a better require.extensions, no one is stopping you. Why haven't you built that yet? Simple: It's silly feature no one actually needs, the only purpose of which is to split the node community and limit the utility of modules you write with it. When no one relies on require.extensions, we'll remove it. Until then, it stays where it is. |
|
@isaacs Would it be sensible - given I just realized this won't work well. Packages could depend on coffee-script for other reasons than loading coffee files at runtime. Maybe write different kind of detection code? Maybe see if there are coffee files without associated js files (indicating they were most likely pre-compiled)? This may be relatively hard to do for existing packages, but wouldn't be too hard at publish time. |
|
I'd like to only mark packages (or rather, nudge authors) that actually use this feature, not just ones that happen to depend on coffee-script. There are some new cool things coming soon that will make this possible :) |
|
@isaacs I assume you're replying from email. See my edit for my a better suggestion. But you might have found something better still. Waiting patiently for the cool things. ;) |
|
i am looking foward to this |
|
@isaacs I don't suppose sending the maintainer of the current require.extensions implementation a pull request would qualify as "building a better require.extensions", would it? Instead of chalking up the lack of an improved system to my disinterest, it might make more sense to assume the simple thing: that I'm writing messages and sending you pull requests not because I'm so disinterested that I've somehow wrapped around right back to motivated action, but rather because I am trying to engage in good faith with the open source process to find a decent solution that will satisfy both you and people who use this feature. Since that hasn't worked, I'm happy to solve this in a way that suits me by - as you say - just writing a better require.extensions. |
Hi,
I ran into some strange behaviour recently when trying to require a file with a .foo.bar style extension that was defined in require.extensions.
tryExtensionsworks fine since it just uses simple string concatenation, butModule.loadonly passes the last part of the extension (eg .bar) to require.extensions, so it barfs, usually with a cryptic parse error because it's trying to read the file as javascript.I figured this qualified as a bug (and so pointed the commit at 0.10) because it's inconsistent behaviour between different parts of the module loader, and I can't imagine a situation where you'd add '.foo.bar' to require.extensions and expect it to not be called.