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

Work even when 'fs' not available for require #164

Closed
wants to merge 1 commit into from

Conversation

lpw
Copy link

@lpw lpw commented Sep 17, 2018

Sometimes the node module 'fs' is not available, so instead of failing completely, this will help 'html-to-text' regress gracefully by changing the one dependent library function fromFile to callback with a specific failure, if called at runtime.

Also, by using a string variable as an argument to require, it helps prevent webpack's static analysis from failing if it happens to try making a bundle in an fs-less environment.

Since it seems many others were also having this problem, I'm raising the PR here instead of just creating another fork with the fs code removed.

and don't let webpack trip on it either (by using string variable as require argument which prevents static analysis/bundling failure).
@moQuez
Copy link

moQuez commented Oct 10, 2018

oh yes.... please merge this!

@mlegenhausen
Copy link
Member

When is fs not available?

@lpw
Copy link
Author

lpw commented Oct 10, 2018

In our particular case we'd like to use html-to-text in an embedded system that has no file system (hence, trying to import fs will fail). Many of the reported problems are for use in the browser (also no fs).

One could argue the merits of doing html-to-text client-side vs server-side, but it's more arguable that html-to-text should not depend on fs as there's no real need (fromFile is just a simple convenience function that could be in the calling code or should be in a separate library instead, to avoid introducing this otherwise unneeded system dependency).

Removing fromFile and its fs dependency at this point might break compatibility since people may already be using it, so this PR just makes fs conditional with a graceful fallback.

@jbwyme
Copy link

jbwyme commented Mar 26, 2019

+1, would like to use this in the browser

mlegenhausen pushed a commit that referenced this pull request Mar 26, 2019
@mlegenhausen
Copy link
Member

Sorry for the long wait. See #171 for the progress of this feature. I will introduce the breaking change cause the upgrade process is trivial and document in the changelog.

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

4 participants