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

Broken with Browserify #50

Closed
leeola opened this issue Feb 20, 2013 · 17 comments
Closed

Broken with Browserify #50

leeola opened this issue Feb 20, 2013 · 17 comments
Assignees
Labels

Comments

@leeola
Copy link

leeola commented Feb 20, 2013

I attempted to use showdown with browserify and ran into a failure. The failure is due to Showdown trying to access the file system, based on the existence of require/etc.

My main question is.. why, why is it even doing this? If we look at the code in question..

//
// Automatic Extension Loading (node only):
//

if (typeof module !== 'undefind' && typeof exports !== 'undefined' && typeof require !== 'undefind') {
    var fs = require('fs');

    if (fs) {
        // Search extensions folder
        var extensions = fs.readdirSync((__dirname || '.')+'/extensions').filter(function(file){
            return ~file.indexOf('.js');
        }).map(function(file){
            return file.replace(/\.js$/, '');
        });
        // Load extensions into Showdown namespace
        Showdown.forEach(extensions, function(ext){
            var name = stdExtName(ext);
            Showdown.extensions[name] = require('./extensions/' + ext);
        });
    }
}

Why is fs even being used here? This seems like a complex solution for something that a simple index.js file would handle just fine. On top of that, if an index.js file was used then Browserify would work out of the box.

@selfcontained
Copy link

Would be nice to have it work w/ browserify, and there are other options aside from removing all server-side code. If there are truly differences between a browser build and what's used in node, you can provide a separate browser version via the browser property in package.json, which browserify respects. Sometimes it takes some organization in dependencies to make these separations clean.

https://gist.github.com/shtylman/4339901

@leeola
Copy link
Author

leeola commented Mar 27, 2013

Well, in this case you wouldn't even need to provide two different versions. This version will work fine in both Browser and Server, it's just that the above code essentially recreates require for absolutely no reason (none that i can figure at least).

For now i'm just concating the js file, but eventually i need to fork this and fix the issue. I can't find a reason for the above filesystem calls :/

@craigspaeth
Copy link

+1

@ryanflorence
Copy link

This also bums me out, anybody have a fork out there already before I go make one?

@ThomasDeutsch
Copy link

+1

@SystemParadox
Copy link

This causes it to fail with webmake as well. Wrapping the require in a try-catch would be sufficient as a quick fix. Not sure if that would help browserify or not.

@erikras
Copy link

erikras commented Apr 17, 2014

+1

@ryanflorence
Copy link

I don't think so, browserify uses static analysis, not runtime evaluation.

On Thu, Apr 17, 2014 at 4:42 PM, Erik Rasmussen notifications@github.comwrote:

+1


Reply to this email directly or view it on GitHubhttps://github.com//issues/50#issuecomment-40771009
.

@bhirt-bpl
Copy link

+1

@ryanflorence
Copy link

Just use marked.

Showdown is clearly not maintained.

On May 21, 2014, at 3:19 PM, bhirt-bpl notifications@github.com wrote:

+1


Reply to this email directly or view it on GitHub.

@gaearon
Copy link

gaearon commented Jul 24, 2014

Same problem with webpack.

@mzabriskie
Copy link

Any progress on this front? It's been a year and a half with no response.

@pikeas
Copy link

pikeas commented Oct 18, 2014

+1 for progress or an update on this...

@aminroosta
Copy link

i have applied the @rjmackay pull requrest in my fork :
https://github.com/aminroosta/aminroosta.github.io

no more browserify or webpack errors .

@tivie tivie self-assigned this Jan 4, 2015
@tivie
Copy link
Member

tivie commented Jan 5, 2015

I'm currently looking into this. Showdown will have a major rewriting as keeping up with the perl version doesn't make much sense now. Since we'll probably move each
extension to its own repository, there will be no reason to call fs.

Regardless, the ideal way to approach the problem is abstracting the
library from the platform it's running on, in a similar fashion to
underscore.js method. This will make it compatible with node and browser
out of the box and, consequently, compatible with browserify. It will also
be easier to make the lib compatible with AMD and commonjs.

@zallek
Copy link

zallek commented Feb 15, 2015

It's fixed in the showdown2 branch.
Note: You now need to instantiate converter using var converter = new Showdown.Converter();

@tivie
Copy link
Member

tivie commented May 15, 2015

fixed in eae5f0e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.