Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

fix: outputPath function overriden by useRelativePath #139

Merged
merged 3 commits into from
Apr 1, 2017

Conversation

economysizegeek
Copy link
Contributor

What kind of change does this PR introduce?

bugfix - The new useRelativePath option code - did not handle the case where you used a function for the outputPath generation.
Did you add tests for your changes?
Yes - I added in a test to make sure that if you use a function for outputPath it is called. If you use the useRelativePath it is ignored.
If relevant, did you update the README?
You don't currently document using outputPath as a function so I wasn't sure I should.
Summary
New code broke old behavior. If you use a function to set the outputPath it was expected that the function would be the decider on where the output of the file is. Recent code took that away:(

Motivation: Recent code broke old behavior. I updated to the current version and my app stopped generating correctly.

Does this PR introduce a breaking change?

No - it restores functionality.

Other information
There weren't any tests for outputPath as a function. I've added one so that in the future the feature will be protected.

@jsf-clabot
Copy link

jsf-clabot commented Apr 1, 2017

CLA assistant check
All committers have signed the CLA.

@joshwiens joshwiens changed the title BUGFIX: outputPath function was being overriden by the new useRelativePath code fix: outputPath function overriden by useRelativePath Apr 1, 2017
@joshwiens joshwiens merged commit 80cdee2 into webpack-contrib:master Apr 1, 2017
@adriancmiranda
Copy link
Contributor

adriancmiranda commented Apr 5, 2017

@economysizegeek Ow! I see and I'm sorry for this, but this fix also broken the outputPath for useRelativePath at this point #L46
Can you verify for me, please?

@economysizegeek
Copy link
Contributor Author

I left that block in tact and didn't modify it from what is happening with relative path. (I'm not sure it made sense to handle an outputpath function with the userelativepath - so i left them mutually exclusive). Could you give a sample config and explain what happens that is wrong?

@adriancmiranda
Copy link
Contributor

adriancmiranda commented Apr 5, 2017

Apparently isn't different, but, if you use path.join foo with ../foo you got foo not ../foo ;-)

@economysizegeek
Copy link
Contributor Author

Just so I could write a test - can you tell me the config.name you have set so I can see all three arugments sent to the join so it resolves to path.posix.join("foo", "../foo", ?) == "../foo"

@adriancmiranda
Copy link
Contributor

@economysizegeek Sorry if I'm slow in answering. I'm working in another approach to useRelativePath. There are still outstanding issues at #135. Honestly, I still don't know how to write the tests more consistently without using a "useless" property just to mock up the issuerContext(Maybe you can help me), but if you want to see the idea, follow the link.

@Jorybraun
Copy link

Jorybraun commented Apr 13, 2017

i'm still having this issue, when I use relative path. It overides the publicpath and appends '../' to the front of the relative folder.

{ test: /\.(png|jpg|gif|svg|eot|ttf|woff|woff2)$/, loader: "file-loader?name=[name].[ext]", query: { useRelativePath: true } },

@Jorybraun
Copy link

Jorybraun commented Apr 13, 2017

  ../fonts/glyphicons-halflings-regular.eot  20.1 kB          [emitted]
                ../img/exclamation-mark.png  2.13 kB          [emitted]
  ../fonts/glyphicons-halflings-regular.svg   109 kB          [emitted]
  ../fonts/glyphicons-halflings-regular.ttf  45.4 kB          [emitted]
 ../fonts/glyphicons-halflings-regular.woff  23.4 kB          [emitted]
../fonts/glyphicons-halflings-regular.woff2    18 kB          [emitted]

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

Successfully merging this pull request may close these issues.

5 participants