lukeed / taskr

A fast, concurrency-focused task automation tool.
MIT License
2.53k stars 74 forks source link

Uncorrect matching behavior of fly.target #230

Closed hzlmn closed 7 years ago

hzlmn commented 7 years ago

Found weird issue.

When sourcing files like ./uikit/uikit.sass uikit-react/uikit-react.sass transforming with fly-sass and then placing compiled files to ./dist/css for example.

It produces ouput like ./dist/css/uikit.css and ./dist/css-react/uikit-react.css instead of all in a single place.

lukeed commented 7 years ago

Can you please show your task?

hzlmn commented 7 years ago

Here is flyfile

const path = require('path')

/**
 * Aliasses
 */
const jn = path.join
const rs = path.resolve

/**
 * Main dirs
 */
const mainPath = jn(__dirname, 'app/ui-kit')
const distPath = rs(__dirname, '../dist')

/**
 * Sass file sources
 */
const sassFiles = [
  jn(mainPath, 'scss/gl-uikit/gl.uikit.scss'),
  jn(mainPath, 'scss/gl-uikit-angularjs/gl.uikit.angularjs.scss')
]
const cssDist = jn(distPath, 'gl-uikit/css')

/**
 * Js sources
 */
const jsFiles = jn(mainPath, 'module/*.js')
const jsDist = jn(distPath, 'js')

const x = module.exports

/**
 * Compile sass sources
 */
x.sass = function * (opts) {
  yield this.source(sasFiles).sass().target(cssDist)
}

/**
 * Compile ESnext sources
 */
x.babel = function * () {
  yield this
    .source(jsFiles)
    .babel({
      preload: true,
      plugins: [
        'add-module-exports',
        'transform-es2015-modules-umd'
      ],
      minified: true
    })
    .target(jsDist)
}

x.default = function * () {
  yield this.serial(['sass', 'babel'])
}
lukeed commented 7 years ago

Thanks!

Are there any * in your directory names?

Also, can you please try without using any path methods. Fly does what you're doing under the hood, there's no need to resolve anything ahead of time.

const main = 'app/ui-kit'
const dist = '../dist'

const sassFiles = [
  `${main}/scss/gl-uikit/gl.uikit.scss`,
  `${main}/scss/gl-uikit-angularjs/gl.uikit.angularjs.scss`
]
const cssDist = `${dist}/gl-uikit/css`

const jsFiles = `${main}/module/*.js`
const jsDist = `${dist}/js`
lukeed commented 7 years ago

There's also a typo in your sass task -- maybe copy/paste error?

Fix sasFiles to sassFiles

hzlmn commented 7 years ago

Nope, same thing only gl.uikit.angularjs.scss placed into target directory It works fine with it

for (var i = 0; i < sassFiles.length; i++) {
  var file = sassFiles[i]
  yield this.source(file).sass().target(cssDist)
}
hzlmn commented 7 years ago

btw, I am sorry for this, it's not urgent, seems like very early in your timezone.

lukeed commented 7 years ago

No problem 😆 I was awake. I just finished traveling & purposefully adopted a new schedule.

Looking into it locally.

lukeed commented 7 years ago

Ok, with my setup, I have gl.uikit.angularjs.css writing to dist and I have the other file writing back to the source directory, so that gl.uikit.scss and gl.uikit.css are siblings.

Can you confirm this?

hzlmn commented 7 years ago

In my case gl.uikit.angularjs.css written to dist, but gl.uikit.css is not written anywhere. Also if i change order of files in sassFiles it behave in a different manner.

lukeed commented 7 years ago

Ok. I think I see a fix. Once I'm more certain, I'll have you paste in the change & confirm before I commit, if you don't mind.

hzlmn commented 7 years ago

Sure, thank you.

lukeed commented 7 years ago

OK. In fly/lib/api/target.js, add this to the end of trims:

const trims = (...).map(...).sort((a, b) => b.length - a.length);

Then at the bottom, when looping thru trims, change o.dir = obj.dir.replace() to o.dir = o.dir.replace()

hzlmn commented 7 years ago

Seems working, thanks.

Just wondering, what a purpose of mapping on dirs like it's really rare case when you want to place same files into two or more diferrent directories, isn't it?

lukeed commented 7 years ago

Cool.

Not really, I do it fairly often. It also doesn't affect anything negatively & it was a Fly 1.x feature. Maybe even 0.x, can't remember.