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

use more specific path for method require #260

Closed
wants to merge 1 commit into from

Conversation

hefangshi
Copy link

We encounter a problem when we upgrade fs-extras from 0.18.3 to 0.30.0, all extra method like fs.move is undefined after we upgrade our service.

Our deploy system is a increment deploy system, so every history file was on the machine.

In 0.18.3 the file path is lib/move.js and in 0.30.0 is lib/move/index.js, they can both by required by using require('./move'), so when we upgrade the fs-extras lib, the new index.js will require('lib/move.js') rather than lib/move/index.js since old file is still exist.

I think it won't hurt anything if we change require path to a more specific path, and will get a better compatibility.

We encounter a problem when we upgrade fs-extras from 0.18.3 to 0.30.0, all extra method like fs.move is undefined after we upgrade our service.

Our deploy system is a increment deploy system, so every history file was  on the machine.

In 0.18.3 the file path is `lib/move.js` and in 0.30.0 is `lib/move/index.js`, they can both by required by using `require('./move')`, so when we upgrade the fs-extras lib, the new `index.js` will require('lib/move.js') rather than `lib/move/index.js` since old file is still exist.

I think it won't hurt anything if we change require path to a more specific path, and will get a better compatibility.
@jprichardson
Copy link
Owner

I don't see how this really changes anything? The require algorithm will look for index.js if it finds a directory.

@hefangshi
Copy link
Author

Yes you are right, and the code is working great at in normal scenes.

The thing is our deploy system is a increment deploy system, so there will be lib/move.js and lib/move/index.js at the same time after we upgrade fs-extras from 0.18.3 to 0.30.0, and the require algorithm will find old lib/move.js be default, and the code was broken.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 31, 2016

@hefangshi This is a problem with your deploy system, not fs-extra IMO. Going to close this for the time being.

@RyanZim RyanZim closed this Oct 31, 2016
@hefangshi
Copy link
Author

It has no harm for fs-extra, so why don't accept it?

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.

3 participants