[TIMOB-24642] - Correct require implementation

[ewanharris]

JIRA: https://jira.appcelerator.org/browse/TIMOB-24642

Note iOS does not like a leading slash on paths, so '/node_modules' in the below is 'node_modules' on iOS

• Decrement i before continuing
• If startDir/path === '/' return early with "/node_modules" to prevent unnecessary loop
• Always appends '/node_modules'
• On iOS correct the path builder to split the array rather than perform a substring on the string

Test case:

1. Download the test app linked in the ticket description
2. Build the app, it should load to a yellow screen and in the console you should see.

[INFO]  boz index.js loaded
[INFO]  boo boo.js loaded
[INFO]  baz index.js loaded
[INFO]  bar bar.js loaded
[INFO]  foo lib/foo.js loaded
[INFO]  foo index.js loaded

[cb1kenobi]

When a path begins with /, it implies a resource, not module. require('/path/to/file.js') is a resource. require('path/to/file') is a module. Module identifiers do not have an extension. Resources require an extension. The initial require() implementation handled / wrong. If you require('/foo'), it should NEVER search node_modules. Not even Node does that.

[sgtcoolguy]

move this comment back down to the line it's referencing below.

[sgtcoolguy]

@ewanharris It'd be useful to add some tests around this in our test suite so we know the issue is fixed, and to verify some of the things we're discussing here.

Copy https://github.com/appcelerator/titanium-mobile-mocha-suite/blob/master/Resources/ti.require.test.js in titanium_mobile/tests/Resources and add some unit tests that verify we don't have an infinite loop. You may need to also override titanium_mobile/tests/Resources/node_modules to include some additional node modules that trigger the bug (right now there's just the abbrev module there: https://github.com/appcelerator/titanium-mobile-mocha-suite/tree/master/Resources/node_modules/abbrev).

As for what @cb1kenobi is talking about, I don't believe we do search for node_modules when the require path is prefixed by ./, ../ or /. This fix is related to gathering the list of possible paths to search for node_modules folders up the directory tree from our current location (in the cases where we should be looking for them).

[sgtcoolguy]

Requesting unit tests be added to verify the bug is fixed.

[ewanharris]

@sgtcoolguy Addressed your comments, added the following tests

1. this ticket
2. validating the caching of modules and module lookup works as expected i.e closest node_modules first
3. Checking what @cb1kenobi had mentioned, which exposed a bug on Android, see TIMOB-24673
4. Unskipped a test that works on android now

[mukherjee2]

Passed FR!
Node Version: 6.10.1
NPM Version: 3.10.10
Mac OS: 10.12.4
Appc CLI: 6.2.0
Appc CLI NPM: 4.2.9
Titanium SDK version: 6.2.0.v20170510080439
Appcelerator Studio, build: 4.8.1.201612050850
Xcode 8.3.2

First tested with 6.0.4.GA. The app hangs. I retried with the fix in this PR, and the app launched and displayed a yellow window. FR passed.