Relative paths inconsistency between 'src', 'build', and 'bin'

[Merott]

Following our discussion about code organisation, I've done some playing around, and noticed an issue with relative paths and how they are affected by the build and bin directory structure.

- src
  |- app
     |- app.js
     |- app.less
  |- assets

If you use a relative path in your less file to reference an image in assets, it would look like this when working out of src:

background: url(../assets/bg.jpg)

When the app builds into the build directory, app.css appears under build/styles/app folder, while assets is build/assets. The above relative URL breaks.

When the app compiles into the bin directory, everything is put into an assets directory, which means that the relative URL will actually work just fine, although it looks up one level and back to where it started in assets.

Obviously the same issue could happen with any sort of relative path and not just in less files. I can't really think of a clean solution right away, all I know is that it needs one, so I'm just putting it out there! :-)

[joshdmiller]

I often run into subtle path problems with Gulp streams. Let me tease out a couple of distinct issues here.

First, a less file at src/app/app.less should compile to build/styles/app.css in the build because the files are supposed to be relative to src/app. If it's not, that's a bug.

Second, paths to assets in LESS or CSS files should always be relative to where the assets are in the build and not where they are in the sources.

Lastly, while these solve most instances of this problem, they certainly don't fix them all. Anything in a subdirectory of src/app will be in a subdirectory in build/styles but not in bin/assets. We can change that, but at the expense of developer insight.

However, I do have an idea to fix it. I'll post it later - it's hard to type code on a mobile.

[Merott]

Forgive me, this happened purely because of my messing about with the directory structure. You are right, it builds to build/styles/app.css.

paths to assets in LESS or CSS files should always be relative to where the assets are in the build and not where they are in the sources.

I don't agree with that though, because:

1. It would be confusing if relative paths didn't match up during development. A developer shouldn't have to worry about where their assets end up in the build and bin directories.
2. Development IDEs such as JetBrains WebStorm validate paths, and provide help with navigating directories. If a path is not found, it is highlighted as an error. Most IDEs allow these to be suppressed, but I see this as a useful feature.
3. If I wanted to switch to a different build system, I should only have to make minor build-related changes, and exceptional changes to the source code.

What are your thoughts?

[joshdmiller]

I would disagree for a couple reasons.

Resources will require different paths between build and compile. This is unavoidable because we are organizing by feature. A CSS file could be at src/app/common/editor/toolbar/toolbar.css, which in the build will be at build/styles/common/editor/toolbar/toolbar.css so that we can find it when using the inspector. But it can't remain nested after the compile because it's concatenated with files from different directories. These are competing priorities, to be sure.

Speaking of organizing by feature, if we had a reference to an icon in the above file, the path would be ../../../../assets/images/icons.png. Setting aside that I don't think that's any easier to follow, the component is no longer drag-and-drop reusable because it had a path dependent on its position in the filesystem.

I agree in principal that Warlock should stay out of the way as much as is possible, but without being completely prescriptive in its structure, this isn't completely possible because some inevitable variation must be accounted for between applications. And that brings us to the only solution of which I can think that doesn't compromise on existing features: running the files through templating:

.whatever {
  background-image: url(<%= assets %>/images/icons.png);
}

This solves the build, but the compile takes its files from the build, at which point the template had already been compiled with the wrong path. The solution, I think, is to allow flows to have multiple endpoints. E.g.:

warlock.flow "webapp-javascript",
  source: [ "<%= globs.source.js %>" ]
  source_options:
    base: "<%= paths.source_app %>"
  dest:
    "webapp-build": 
      path: "<%= paths.build.scripts %>"
      at: 30       # priorities, like with merges
    "webapp-compile": 
      path: "<%= paths.compile.assets %>"
      at: 100

.add( 10, "lint", jshint )
.add( 40, "concat", concat )
.add( 50, "uflify", uglify )

# this doesn't actually change
warlock.flow "webapp-coffeescript"
  source: [ "<%= globs.source.coffee %>" ]
  source_options:
    base: "<%= paths.source_app %>"
  merge: "webapp-javascript@20"

Running webapp-build would stop at a certain point and output to that destination, while running webapp-compile would output only at the end (or perhaps both times, like default). This has other advantages as well, such as reducing the total number of flows, simplifying names of flows, and making it easier to follow the build process end-to-end.

We've obviously now required a variable in the CSS, making it more difficult to switch away from Warlock, which is a small disadvantage, but we do gain readability and flexibility I think.

What do you think?

---

PS (completely unrelated): You've been super helpful. If you'd like to be "official" and added to the ngbp/warlock org on github, shoot me an email (yours isn't on your github page).

Post-PS: It's after 3am. What the hell am I still doing awake?

[Merott]

You're right - that's unavoidable.

I'm still going to insist and try to find a way to keep IDEs happy though... If I have a relative path like ../../../assets/boom.png, WebStorm won't like it any other way, including the template code, and gets quite aggressive towards "errors". If you haven't seen it before, this is it:

It doesn't normally highlight the project tree like that if a path is wrong. In this instance, it sees the template code as syntax error. I could suppress that message, but I'm too arrogant :)

Here's an idea. I must admit the suggestion does make some assumptions, but I'm going to leave them up to you to validate. Let's assume this is the path to our file as it appears in the source:

.whatever {
  background-image: url(../../../assets/boom.png);
}

With some regex find/replace we can prepare this for the build. Find:

(\.\.\/)*assets/i

and replace with ../assets for build, or with ./assets for compile

If it is too bold of an assumption to assume that any occurrence of ../../assets under the source is a relative path to our assets directory, we could probably extend this logic to verify paths using whatever node tools available, before replacing the text.

---

P.S. I'd love to. An email will be coming your way.

Post-PS: I don't know, but I understand.

[joshdmiller]

I'm not sure how big an assumption that is; I'll have to think about that. I use vim for my development (I'm one of "those" guys) so this is a problem that is foreign to me.

An undocumented feature of Warlock is being able to prevent steps in a flow from running. So let's say this regular-expression-based step was called webapp.styles-to-build.replace-paths, a user who did not want the feature could preempt it in warlock.json:

{
  "prevent": [ "webapp.styles-to-build.replace-paths" ]
}

So as long as it's something most people will want and as long as it's well-documented, things like this aren't too much of a problem. Convention over configuration with an emphasis on customization is a key philosophy of the tool, I think.

That said, we'd still have a problem between build and compile because the first regex already replaced the original source values. We could use a different regex to replace the build value with the compile value, but I predict this becoming very fragile if someone starts changing the default paths or file structure. So perhaps something like the above restructuring of flows is still needed (not really a "restructuring"; more a "refactoring" because existing flows would still work).

[Merott]

Actually, you've sort of documented the prevent feature.

It may seem like a big assumption at first, but I don't think it is anymore, especially if we add the extra step to verify the match as a valid existing relative path.

About your point:

We could use a different regex to replace the build value with the compile value, but I predict this becoming very fragile if someone starts changing the default paths or file structure.

How would replacing occurrences of ../assets with ./assets break something? The thing is that if we run find/replace under source_app, replacing (\.\.\/)*assets/i with ../assets, we'll have already touched all references to assets, so it would be pretty safe to assume at this stage that any occurrence of ../assets under build has already been touched by the regex, and should be ./assets when it goes under bin.

My point is, if it's going to break, it'll break during build. Or am I missing something?

[joshdmiller]

Because:

{
  "paths": {
    "source_assets": " src/static"
  }
}

Boom! Broken.

While we could user the path variables themselves to build the expressions, I think it will get very fragile as people move parts about. The source assets, build assets, compile assets, and all their surrounding and containing directories are configurable. I don't see how we can reliably ensure that kind of replacement.

[Merott]

Right.

I think this could be my last suggestion before I give up and suppress those warnings. A hybrid approach...

Early on in the flow, and before any files are processed, for every file we determine what the relative path to the assets folder would look like. Take this file for example:

/* <project_folder>/src/app/common/widget/widget.less */
.widget-container {
  background-image: url(../../../misc/static/images/widget-bg.png);
}

And let's assume source_assets is defined as:

{
  "paths": {
      "source_assets": "<%= paths.source %>/misc/static",
  }
}

Based on <project_folder>/src/app/common/widget/widget.less and <%= paths.source %>/misc/static, we can determine the relative path to be ../../../misc/static.

Without using a regular expression, we can do a find/replace on widget.less, and replace ../../../misc/static with '<%= assets %>', preparing the file to run through templating.

After that, the rest of the flows can be implemented as you suggested. Being a hybrid approach, it also has the benefit of allowing the user to directly use template code, if that's how the user prefers to do it.

Doable?

[joshdmiller]

It's probably doable, but I'm still concerned with fragility. If you want to take a stab at implementing it, go for it. Otherwise, we can keep it open and circle back to it later.

Being a hybrid approach, it also has the benefit of allowing the user to directly use template code, if that's how the user prefers to do it.

You mean instead of using the template cache?

[Merott]

I mean, the user can do either this:

.widget-container {
  background-image: url(../../../misc/static/images/widget-bg.png);
}

Or this:

.widget-container {
  background-image: url(<%= assets %>/images/widget-bg.png);
}

If they specify the path using a relative path, it gets changed on the fly to <%= assets %> at the beginning of the flow. If they specify it using <%= assets %>, then it won't get touched.

I think at this point we should probably just go with your suggestion of running files through templating, and requiring assets to be referenced using <%= assets %>.

I'd be happy to try and implement that, but I'm not sure how to do that in Warlock. I presume every file, regardless of its type (?) under source_app will need to run through templating? And that it needs to be done early on?