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

Support multiple destinations in copy-files.js #1055

Open
darylcober opened this issue Feb 20, 2019 · 7 comments
Open

Support multiple destinations in copy-files.js #1055

darylcober opened this issue Feb 20, 2019 · 7 comments

Comments

@darylcober
Copy link

darylcober commented Feb 20, 2019

I'm submitting a feature request

Current behavior:
The copy-files method only supports a single destination. Because the instruction list is normalized you cannot add duplicate destinations by copying the same file twice. My particular application requires some files to be copied to multiple locations for different builds. In order to achieve this I have to have multiple copy-file routines in my code base.

  • What is the expected behavior?

It would be beneficial to have the copy-files method accept an array of destinations for each instruction instead of a single string. The method would copy each file to each location in the instruction array, thus eliminating the need to replicate the copy-file routine.

  • What is the motivation / use case for changing the behavior?
    Increased flexibility in the build process, lower the need for hacks for special requirements, and a cleaner code base.
@3cp
Copy link
Member

3cp commented Feb 21, 2019

cli gives you all the tasks files in aurelia_project/tasks, it's designed for user to customize the tasks.

The cli logic itself does NOT implement copy files logic. It's in your local aurelia_project/tasks/copy-files.js.

To support "copyFiles": {"source/*": ["target1", "target2"]}, you can do following in esnext (need some minor adjustment for typescript). You need npm i -D merge2 or yarn add -D merge2.

import gulp from 'gulp';
import path from 'path';
import changedInPlace from 'gulp-changed-in-place';
import project from '../aurelia.json';
import merge2 from 'merge2';

export default function copyFiles(done) {
  if (typeof project.build.copyFiles !== 'object') {
    done();
    return;
  }

  const instruction = getNormalizedInstruction();
  const files = Object.keys(instruction);

  // Use merge2 to wait on all streams
  return merge2(files.map(file => {
    let targets = instruction[file];
    if (!Array.isArray(targets)) targets = [targets];

    let stream = gulp.src(file)
      .pipe(changedInPlace({ firstPass: true }));

    targets.forEach(target => {
      // Yes, you can pipe same stream to multiple gulp.dest
      stream = stream.pipe(gulp.dest(target));
    });

    return stream;
  }));
}

function getNormalizedInstruction() {
  const files = project.build.copyFiles;
  let normalizedInstruction = {};

  for (let key in files) {
    normalizedInstruction[path.posix.normalize(key)] = files[key];
  }

  return normalizedInstruction;
}

I am not sure if it's a good idea to put this into our default copy-file task file (cc @JeroenVinke @EisenbergEffect), as you can see that only experienced gulp users can understand what's going on here. But we can certainly add this recipe to our doc.

@3cp
Copy link
Member

3cp commented Feb 21, 2019

There is better solution. Personally I am against adding features to any software tool, because minimum feature set actually can result better flexibility and simplicity. Sadly the whole industry had drifted away from this old unix wisdom.

Here is a solution, imagining cli doesn't provide copyFiles field in aurelia.json.

Same copy-files task file. Get the job done in plain gulp way, while provides you maximum flexibility.

import gulp from 'gulp';

function copyFonts() {
  return gulp.src('some/fonts/*')
    .pipe(gulp.dest('dist1'))
    .pipe(gulp.dest('dist2'));
}

function copyImages() {
  return gulp.src('some/imgs/*')
    .pipe(gulp.dest('dist1'))
    .pipe(gulp.dest('dist2'));
}

export default gulp.parallel(
  copyFonts,
  copyImages
);

@darylcober
Copy link
Author

Thanks for responding.
I have used both of the above solutions and both work equally well.

However, both require me to make modifications to the "out of the box" Aurelia platform.
While I certainly can do that, I believe that since you already provide a hook with the copy-files method, to help achieve flexibility in custom builds, having the copy-files method support multiple destinations would just be so much better in my eyes.

It would also be beneficial for "power" users to have a pre-build-file-copy (existing copy-files) and a post-build-file-copy. Many of us are building very complex solutions and having these simple tools available that follow all the existing Aurelia doctrines makes the Aurelia package all the more attractive to us.

Just my opinion ...
Daryl

@3cp
Copy link
Member

3cp commented Feb 21, 2019

I understand, thanks for the feedback.

The solution1 might go into cli default offering. But I don't think we will provide default setup for pre-build/post-build, because

  1. it's confusing to light/medium users.
  2. for complex apps, there is no one solution fitting all. You inevitably need to customise the flow, which also means it's mandatory for the advanced users to master the tool chain in place (gulp in this case). (BTW, good luck if the tool chain is webpack :-))

@darylcober
Copy link
Author

If you go forward to implement solution1 you may consider the following changes I make to my custom solution :


function getNormalizedInstructions() {
  const files = project.build.copyFiles;
  let normalizedInstruction = {};

  for (let key in files) {
    let instruction = files[key]
    if ( typeof (instruction) === 'string') {
      instruction = [instruction]
    }
    normalizedInstruction[path.posix.normalize(key)] = instruction;
  }

  return normalizedInstruction;
}

Simple check for string instruction ... allows user to use either a simple string or array as destination

Cheers,
Daryl

@3cp
Copy link
Member

3cp commented Feb 22, 2019

allows user to use either a simple string or array as destination

The solution1 covered that here:

if (!Array.isArray(targets)) targets = [targets];

@darylcober
Copy link
Author

yup ... sorry .. I missed that :)

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

No branches or pull requests

2 participants