Expose require before initializing entries

[tellnes]

If you, from an entry file, tries to require a module from an external bundle which in turn requires a module from the first bundle, it breaks.

This fixes this by exposing the newRequire function before any entires are executed.

Side effects:

• fixes externalRequireName to detect correct name from other bundles
• lets you have externals and standalone modules at the same time

One major thing to note is that this changes the number of arguments passed to the prelude. That may break some modules and is breaking browser-unpack. Please see browserify/browser-unpack#13 for details.

[terinjokes]

Changing the prelude might be a breaking change here (and in Browserify).

[tellnes]

@terinjokes The breaking change is the number of arguments that it accepts. If that is a change that we can not do, then that can be handled by using an object to define the values.

But I don't think number of arguments should be how browser-unpack checks if its input is a browser-pack output or not. By my opinion it should be changed to eg. a special code commit or maybe it could start with a string statement similar to how 'use strict' works.

[terinjokes]

@tellnes It's less about browser-unpack, and more of a signal to users who have created custom preludes that they might want to look at the changes.

[tellnes]

@terinjokes This can probably be fixed without that breaking changes, but in a more ugly way. I can probably create a pr which tries to do that, but I'm not going to wast time on if not somebody who has commit access says he or her are willing to look at it afterwards. (most of my PRs to Browserify related modules goes unanswered)

Also, we do have major version numbers for a reason. This also fixes the bug when you want to rename the global variable and use multiple bundles. I don't see how that is fixable without providing more information to the prelude.

[terinjokes]

@tellnes It was just a comment asking if it's something we should consider before merging this in, so we can communicate the changes appropriately to end users. I'm not saying we shouldn't merge it, that's why we have major versions numbers (and why Browserify has gone through 13 of them already!). Otherwise it looks fine to me.

I have commit (and publish) bits to many browserify repos, but as you note, not this one.

[tellnes]

@terinjokes Sorry, nothing personal. I was probably a bit rude in my last comment. Looking forward to solving the problem in one way or another.

[terinjokes]

@tellnes no problems, I'm looking at doing some changes to this module, but wanted to clean up the pull requests list first. 😄

@jmm @substack Either of you mind taking a look? This seems reasonable to me.

[jmm]

@terinjokes Thanks for the ping -- I'll try to find time to take a look. I'm not a collaborator here either though, so I could only offer an opinion.

A couple of quick things based on skimming the comments:

@tellnes

lets you have externals and standalone modules at the same time

Sorry, I haven't looked at the code yet -- do you mean that you could do something like this:

browserify("./entry.js", {
  standalone: "whatever"
})
.require("x")

And then do require("x") from outside that bundle?

If so, that came up previously and got negative feedback from a maintainer:

#45
#51

But I don't think number of arguments should be how browser-unpack checks if its input is a browser-pack output or not. By my opinion it should be changed to eg. a special code commit or maybe it could start with a string statement similar to how 'use strict' works.

I haven't yet looked into the issue with prelude arguments that you're discussing here, but the other idea sounds like what I was proposing in browserify/browserify#1151. Including it for reference, though it may no longer be the best solution to the original problem I was trying to solve.

[tellnes]

@jmm The issue I'm trying to resolve is when you are splitting your bundles into multiple files (eg. with factor-bundle). If you have a complex require tree which references modules both directions and is using entries, it does not work.

What I'm changing is that I defines the global require function inside the prelude. Changing this makes those two features independent of each other and therefore has the side effect to fixing that bug.

The old behavior could probably easy be implemented if it is desirable, but I don't see any reason to add extra code for it when it (by my opinion) just reimplements a bug that (as I can see) has no backwards compatibility issues.

[tellnes]

@zertosh Hi. I see that you have done some changes to this module recently. Any change to get this or something like that fixes the attached test case merged any time soon?