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

Update to resolve relative file path based on curDir #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexkaraman85
Copy link

When getting relative path for '/module/img/sample-img.png'

path.relative( srcDir, srcFile )

returns 'sample-img.png'

path.relative(curDir, srcFile)

returns 'img/sample-img.png'

This change allows for path that is rooted to the same structure with only new parent directory.
I got to this point by using cartero-node-hook.getAssetUrl function, which is returning 'fingerprint/sample-img.png' instead of '/fingerprint/img/sample-img.png'.

Please forgive me if this is the wrong place for this update. Im also not sure about why you chose to compare agaist srcDir? Could you please let me know.

When getting relative path for '/module/img/sample-img.png' 
```
path.relative( srcDir, srcFile )
```
returns 'sample-img.png'

```
path.relative(curDir, srcFile)
```
returns 'img/sample-img.png'

This change allows for cartero-node-hook to return a path with just the module directory updated to the fingerprint.
@dgbeck
Copy link
Member

dgbeck commented Feb 13, 2015

Hi @alexkaraman85 !

I'm having a tough time understanding the exact issue that this is solving. Let's try this approach.. can you give an example of a case where behavior of path-mapper is wrong without this change but correct with this change? For example:

pathMapper = require( 'path-mapper' );

var dstFilePath = pathMapper( '/tmp/test.txt', { '/tmp' : '/new_tmp' } );
console.log( dstFilePath ); // should output XYZ, but outputs ZYX (just examples)

Thanks!

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.

2 participants