This repository has been archived by the owner. It is now read-only.

Skip duplicate configurations. #382

Merged
merged 1 commit into from Jun 27, 2014

Conversation

Projects
None yet
10 participants
@boushley

boushley commented Jun 11, 2014

This should resolve #289.

This also detects a bad configuration where multiple blocks attempt to write
to the same destination with different sources.

I realize #324 is already a PR for this, but the method taken in that PR is O(n^2) and this avoids that extra looping. This PR also has tests.

Aaron Boushley
Skip duplicate configurations.
This should resolve #289.

This also detects a bad configuration where multiple blocks attempt to write
to the same destination with different sources.
@XhmikosR

This comment has been minimized.

Collaborator

XhmikosR commented Jun 19, 2014

/CC @yeoman/yeoman-contributors

This seems to be in better shape compared to #324. Can someone review this?

@sindresorhus

This comment has been minimized.

Member

sindresorhus commented Jun 23, 2014

@XhmikosR i don't have enough comprehension of the codebase to be able to judge this, but I trust your judgement.

@boushley

This comment has been minimized.

boushley commented Jun 26, 2014

@XhmikosR @sindresorhus It looks like this has been chosen in favor of the alternate PR. What else needs to happen for this to land?

Anything I can do to help move this forward?

sindresorhus added a commit that referenced this pull request Jun 27, 2014

@sindresorhus sindresorhus merged commit 708ffca into yeoman:master Jun 27, 2014

1 check passed

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

This comment has been minimized.

Member

sindresorhus commented Jun 27, 2014

Landed. Thanks @boushley :)

@sindresorhus

This comment has been minimized.

Member

sindresorhus commented Jun 27, 2014

Published as 2.3.0

@boushley

This comment has been minimized.

boushley commented Jun 27, 2014

Thanks!
On Jun 26, 2014 5:10 PM, "Sindre Sorhus" notifications@github.com wrote:

Published as 2.3.0


Reply to this email directly or view it on GitHub
#382 (comment).

@htanjo

This comment has been minimized.

htanjo commented Jun 27, 2014

Awesome! This works perfectly in my project.
Thanks!

@schmod

This comment has been minimized.

schmod commented Aug 19, 2014

This is great. Thanks for the PR.

In the interest of keeping things DRY, is there a better way to manage "multi-page" apps with usemin?

While the error-checking in this version is greatly appreciated, it'd be even better if we didn't have to carefully keep all of our blocks in sync.

Some options might be:

  • Declare our blocks in a partial, and splice it into the main HTML before useminPrepare runs. (Cons: Makes localhost testing difficult)
  • Add a configuration option to merge file lists when we specify multiple build targets with the same name (Cons: Could be unpredictable)
@boushley

This comment has been minimized.

boushley commented Aug 19, 2014

@schmod That's actually exactly what I am doing. I'm using Metalsmith as a static site generator. I have a partial that lists the script block to be included on each page. It gets rebuilt quickly so local development is still speedy, and I'm not trying to ensure multiple blocks are in sync.

I added the code here that ensures that they are the same, just to be sure people didn't accidentally define blocks that would cause a weird and confusing bug when trying to debug it.

@albertpeiro

This comment has been minimized.

albertpeiro commented Oct 3, 2014

I'm not sure I have the same use case but here's the issue I'm facing, very related to this:

index.html:

<!-- build:css(.tmp) styles/index.css -->
    <link rel="stylesheet" href="styles/main.css">
<!-- endbuild -->

signin.html

<!-- build:css(.tmp) styles/signin.css -->
    <link rel="stylesheet" href="styles/main.css">
    <link rel="stylesheet" href="styles/signin.css">
<!-- endbuild -->

Now my just scaffolded app gruntfile:

useminPrepare: {
      html: {
        src: ['<%= yeoman.app %>/index.html']
      },
  options: {
    dest: '<%= yeoman.dist %>',
    flow: {
      html: {
        steps: {
          js: ['concat', 'uglifyjs'],
          css: ['cssmin']
        },
        post: {}
      },
    }
  }
},

A few months ago (latest version) I could modify useminPrepare to:

  useminPrepare: {
        options: {
            dest: '<%= config.dist %>'
        },
        html: '<%= config.app %>/{index,signin}.html'
    },

And it would generate index.12345.css and signin.12345.css and correctly replace them in the .html files as needed. This doesn't work anymore and it throws me:

Fatal error: Different sources attempting to write to the same destination:
 {
     "dest": "dist/styles/vendor.css",
     "src": [
         "bower_components/bootstrap/dist/css/bootstrap.css"
     ]
 }
   {
     "files": []
 }

How can I achieve this with the newest version? Thank you.

@richardwestenra

This comment has been minimized.

richardwestenra commented Nov 2, 2014

+1. I'm using generator-jekyllrb with custom designs for different pages but with shared main.css, modernizr.js and main.js files across the site. After updating, I'm finding that it will no longer let me scan multiple html files to find all the custom css/js files:

useminPrepare: {
  options: {
    dest: '<%= yeoman.dist %>'
  },
  html: '<%= yeoman.dist %>/**/*.html'
},

If I can't get it to scan all the html files and ignore duplicates, is there any way to bypass useminprepare and manually tell it which files to use?

The way I'm doing it at the moment is to create a throwaway userminprepare.html file which just contains references to all of my js/css, and I run useminprepare on it before deleting it with the clean task when deploying. It works and it's fast but it's not very DRY.

@boushley boushley deleted the boushley:skip-duplicates-289 branch Nov 6, 2014

@plrthink

This comment has been minimized.

plrthink commented Dec 10, 2014

+1. Is there some updates?

@mCarini86

This comment has been minimized.

mCarini86 commented Dec 11, 2014

+1 Any Update?

@ruyadorno

This comment has been minimized.

Member

ruyadorno commented Dec 11, 2014

@plrthink @mCarini86 we just got PR #496 in time for the v3.0.0 release, with this new version it should be possible to use the --force option and have usemin working with duplicate blocks like before.

This fixed my use case, similar to what @richardwestenra described (sharing main.css across multiple html pages), the only downside is that I have to use the force option now...

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