Proposal: Add host option to override node fs and path modules#174
Closed
barak007 wants to merge 1 commit intopillarjs:masterfrom
Closed
Proposal: Add host option to override node fs and path modules#174barak007 wants to merge 1 commit intopillarjs:masterfrom
barak007 wants to merge 1 commit intopillarjs:masterfrom
Conversation
Contributor
|
Sorry, this is not the correct implementation for all the reasons from the previously closed PRs and issues on this. Please move the discussion/proposal to #160 . If someone can provide a proposal that addresses these, that would be the way to move forward. I'm actually working on a security fix currently that would complete rewrite which fs methods are called, even apart from the changes PR #125 makes. This PR would make those types of changes no longer backwards compatible as our exact usage of the internal fs becomes public as folks will end up only passing in the bare minimum to make their use case work. See past discussion #149 as well. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
I have a use-case that requires me to serve files from memory in my tests.
Currently, I use
express.static()fromserve-staticwhich uses this package. Now, I want to keep the same API but enable passing an option to override the default host file-system and path resolution.There is an issue (expressjs/serve-static#83) in
express-staticthat requests the same thing but hasn't seen much activity.About the tests missing from this pull request.
Since there is no changed behavior, all of the current tests are passing. What I would really want to do is wrap all the existing test-cases in a factory and re-run them using an external host.
Would you have any objections to this test plan? Would love to get this in.