Skip to content

Conversation

mtth
Copy link
Contributor

@mtth mtth commented Oct 14, 2015

Path keys corresponding to skipped paths (where the value is false)
did not get expanded, so only module IDs were supported. This lets us
skip relative paths as well.

if (replacements[key] === false) {
return shims[key] = __dirname + '/empty.js';
val = __dirname + '/empty.js';
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow existing code style and spacing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, updated the indent width. There aren't any other else statements in the code, so I indented it like your catch statements. Let me know if this isn't what you had in mind.

Keys corresponding to skipped paths (i.e. where the value is `false`)
did not get expanded, so only module IDs were supported. This lets us
skip relative paths as well.
@defunctzombie
Copy link
Collaborator

Just to make sure I understand this change. Previously a browser field value of:

browser: {
    "./local-file.js": false
}

Would not work when required from another module (require(foo/local-file)) because the path would not be expanded? And this PR fixes that behavior?

@mtth
Copy link
Contributor Author

mtth commented Oct 14, 2015

This is for requires inside the same package. Assuming the following folder structure (a bit contrived for simplicity):

  • index.js (with require('./lib/hide'))
  • lib/
    • index.js (with require('./hide'))
    • hide.js (should be always skipped for browser builds)

In this case, we have to set both ./hide and ./lib/hide to false in package.json (which wouldn't be the case if they were just relocated):

browser: {
    "./hide": false,
    "./lib/hide": false
}

With this change, skipped keys behave the same as relocated ones. Hope this makes sense.

@defunctzombie
Copy link
Collaborator

Makes perfect sense. Great fix!

defunctzombie added a commit that referenced this pull request Oct 14, 2015
@defunctzombie defunctzombie merged commit 42f96b2 into browserify:master Oct 14, 2015
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.

2 participants