Skip to content

Check for module with .js in the name before attempting to resolve file#667

Merged
petekanev merged 1 commit intomasterfrom
pete/require-module-with-js-in-name
Jan 3, 2017
Merged

Check for module with .js in the name before attempting to resolve file#667
petekanev merged 1 commit intomasterfrom
pete/require-module-with-js-in-name

Conversation

@petekanev
Copy link
Copy Markdown
Contributor

@petekanev petekanev commented Dec 28, 2016

Moved around logic for resolving modules when requiring to first check if the resolved file is an actual file, or we are trying to load a directory.

This is necessary when dealing with folders that may contain character sequences that look like file extensions, (hash.js is a module, not a standalone script) while not breaking https://github.com/NativeScript/common-runtime-tests-app/blob/master/Require/index.js#L204 as resolution for directories will succeed only if they contain a package.json or an index.js

Tests added at NativeScript/common-runtime-tests-app#21

Addresses #666

@ns-bot
Copy link
Copy Markdown

ns-bot commented Dec 28, 2016

💚

@Plamen5kov
Copy link
Copy Markdown
Contributor

https://nodejs.org/api/modules.html#modules_all_together
the loading order is implemented following this spec.

@petekanev petekanev force-pushed the pete/require-module-with-js-in-name branch from b2bf115 to 1816ea6 Compare January 3, 2017 08:33
@ns-bot
Copy link
Copy Markdown

ns-bot commented Jan 3, 2017

💚

@petekanev petekanev merged commit 0f815ee into master Jan 3, 2017
@petekanev petekanev deleted the pete/require-module-with-js-in-name branch January 3, 2017 13:34
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.

3 participants