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

Resource paths now resolved relative to html file #1

Merged
merged 1 commit into from Jan 9, 2017

Conversation

Projects
None yet
3 participants
@NeekSandhu
Contributor

NeekSandhu commented Jan 6, 2017

sri generation failed in my case where I had complex directory structure like this:

myApp
|-- gulpfile.js
|-- core
|   |-- scripts
|   |   `-- main.js
|   |-- styles
|   |   `-- main.css
|   `-- index.html
`-- hosts
    |-- electron
    |   |-- scripts
    |   |   `-- main.js
    |   `-- index.html
    `-- webapp
        |-- scripts
        |   `-- main.js
        `-- index.html

Upon debugging index.js I found paths to resources were being resolved relative to gulpfile or cwd (not sure). After this patch they are now resolved relative to html file in question.

Resource paths now resolved relative to html file
sri generation failed in my case where I had complex directory structure like this:
`
myApp
|-- gulpfile.js
|-- core
|   |-- scripts
|   |   `-- main.js
|   |-- styles
|   |   `-- main.css
|   `-- index.html
`-- hosts
    |-- electron
    |   |-- scripts
    |   |   `-- main.js
    |   `-- index.html
    `-- webapp
        |-- scripts
        |   `-- main.js
        `-- index.html
`

Upon debugging `index.js` I found paths to resources were being resolved relative to `gulpfile` or `cwd` (not sure). After this patch they are now resolved relative to `html` file in question.
@codecov-io

This comment has been minimized.

codecov-io commented Jan 6, 2017

Current coverage is 100% (diff: 100%)

Merging #1 into master will not change coverage

@@           master    #1   diff @@
===================================
  Files           1     1          
  Lines          61    61          
  Methods         0     0          
  Messages        0     0          
  Branches        0     0          
===================================
  Hits           61    61          
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last update 4ab8ff4...249f1fc

@macedigital

This comment has been minimized.

Owner

macedigital commented Jan 9, 2017

@NeekSandhu, yes you're absolutely right. It's an undocumented "intended behavior" for folder structures like the one below (so no matter how deep html files are nested, referencing assets/styles/main.css works):

root-folder
|-- gulpfile.js
|-- assets
|   |-- scripts
|   |   `-- main.js
|   |-- styles
|   |   `-- main.css
`-- html
    |-- section
    |   |-- sub-section
    |   |   `-- index.html
    |   `-- index.html

In order to accommodate to your use case, I'll make some modifications, and introduce a configurable option relative: true. After doing so, relative path resolution should work as well.

If you have the time, please test it out (either from git, or wait until v1.2.0 release is out).

@macedigital macedigital merged commit bd27a2b into macedigital:master Jan 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@NeekSandhu

This comment has been minimized.

Contributor

NeekSandhu commented Jan 10, 2017

Great, sounds good. But just out of curiosity, what kind of project uses the structure you mentioned? I've never seen that before.

In my case its because my app is delivered through iframe in browser and webview in electron.

@macedigital

This comment has been minimized.

Owner

macedigital commented Jan 10, 2017

It's a result of using static site generators, e.i. Hugo, which will create nested folder structures containing 'index.html' files.

@NeekSandhu NeekSandhu deleted the NeekSandhu:patch-1 branch Jan 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment