Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browserify compatibility fixes #61 #96

Closed
wants to merge 5 commits into from
Closed

Browserify compatibility fixes #61 #96

wants to merge 5 commits into from

Conversation

rhalff
Copy link

@rhalff rhalff commented Jun 24, 2014

This pull request is meant to fix #61

I'm also using request which uses this module:

https://github.com/mikeal/request/blob/master/request.js#L17

Proof current code fails:

browserify test.js | node
[stdin]:1335
      content = fs.readFileSync(file, 'ascii'),
                   ^
TypeError: Object #<Object> has no method 'readFileSync'
    at Mime.load ([stdin]:1335:20)
    at Object.<anonymous> ([stdin]:1383:6)
    at Object.XCvoj2 ([stdin]:1409:4)
    at s ([stdin]:1:220)
    at [stdin]:1:271
    at Object.<anonymous> ([stdin]:1416:12)
    at Object../mime ([stdin]:1506:4)
    at s ([stdin]:1:220)
    at e ([stdin]:1:391)
    at [stdin]:1:409

Difference between the original patch:

  • additional parse method, which includes the map merging from @dominictarr
  • extracted the definition of Mime into a seperate file so it can be required
    without doing any initialization stuff.

I've added an extra check within the test to see if fs has the readFileSync method,
a bit lame, but that part is using the load() method and otherwise the test will fail. (browserify test.js | node)

The build script does the same thing as the one from @dominictarr, I've just created
the extra parse method to avoid the duplication and it might come in handy by itself.

Note: If any code does call mime.load() within the browser it will fail with the original error, on the
server-side it just behaves like normal.

To clarify browserify; there is no harm in fs being required, but it will just be an empty object:

@see https://github.com/substack/node-browserify/blob/master/lib/builtins.js#L12

Also notice that the types/*.types files can just be considered source files now and could be added to .npmignore

Since quite a lot of packages depend on node-mime it would be nice if it would just compile without having to do any manual overwrites while using browserify.

@Fishrock123
Copy link

Going to leave this here for those who already want a solution: The express team's mime-types.

@broofa
Copy link
Owner

broofa commented Nov 27, 2014

Please refer (and comment on) #107 (Thanks, and my apologies for dragging my heels on this!)

@broofa broofa closed this Nov 27, 2014
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.

None yet

3 participants