Skip to content
This repository has been archived by the owner on Apr 20, 2018. It is now read-only.

enhanced source path resolution for useminPrepare #306

Closed
wants to merge 8 commits into from

Conversation

eli-collins
Copy link

This set of commits make a number of changes to how source urls are resolved,
with the hope of making things more flexible and handling some edge cases.
I tried to change things in a way which should be backwards-compatible
with existing third-party flow plugins, while providing a number of new features:

  • the context object now exposes a context.resolveInFile(fname) helper (provided by the ConfigWriter), which flow plugins should use to convert a filename to it's path
    (rather than using path.join(context.inDir, fname)). This single change
    allows source resolution to be moved back into ConfigWriter, where a
    number of new features have been added...
  • the root option can now be an array, which will be searched in order.
    (this is similar to PR useminPrepare accepts array of root directories #296, but the changes here are made to lib/configwriter,
    rather than lib/config/concat, and should work for all plugins).
  • source urls are now resolved against the entire block searchpath,
    all the root dirs, and then all the file searchpath (this clears up
    a few FIXMEs within the code).
  • I've added a warnMissing option, which causes ConfigWriter to issue a warning/error
    for any source references it can't resolve (ala issue Warn on missing files #260). This defaults to off,
    so as not to break things for people.
  • finally, I added a resolveSource() option, which allows applications
    to completely override how source urls are resolved... this allows handling of
    applications with complex url rewrite rules, runtime template insertions
    within the source url, or any other border case not covered by the default behavior.

I'm still testing this on my own projects, and given that I'm not familiar
with Mocha, don't have any unittests for this features written yet...
but I'm willing to give it a try if there's interest in merging this PR.

edit: All of these features should now have unittests.

@debuggerpk
Copy link

this is actually very very useful , especially when working with django

@eli-collins eli-collins reopened this Mar 18, 2014
@eli-collins
Copy link
Author

Thanks! I'm in a similar boat with a pyramid project that has some rather complex URL mappings. I'm crossing my fingers that this gets accepted eventually, or something with equal flexibility. Until then, let me know if you run into any bugs in the patch (assuming you're actively using the fork :)

@eli-collins
Copy link
Author

I've updated my fork to include the all changes merged into usemin since I made this pull request. I'd really love if these features could make it into usemin, as I'm relying on them in production already :)

If someone could let me know if there's anything I can do to make this patch more acceptable, or if there are parts which need explanation?

@sindresorhus
Copy link
Member

@eli-collins this is really waiting on #313 (comment) If you or anyone you know would be interested let us know. I currently don't have time or enough grasp of the codebase to handle this.

@HiroAgustin
Copy link

Jus to tag some issues here: #260, #404 and #452 are related to this PR.

@stephanebachelier
Copy link
Collaborator

@HiroAgustin This PR is quite old now. Maybe it should be splitted into differents parts.
The first issues you mentioned should be related to usemin complaining about missing files

@stephanebachelier
Copy link
Collaborator

closing it in favor of #518 which is the same work rebased on master with some additions.

@stephanebachelier
Copy link
Collaborator

Thanks @eli-collins !

stephanebachelier added a commit to stephanebachelier/grunt-usemin that referenced this pull request Feb 24, 2015
stephanebachelier added a commit to stephanebachelier/grunt-usemin that referenced this pull request Apr 7, 2015
stephanebachelier added a commit to stephanebachelier/grunt-usemin that referenced this pull request Apr 7, 2015
stephanebachelier added a commit that referenced this pull request Apr 12, 2015
stephanebachelier added a commit to stephanebachelier/grunt-usemin that referenced this pull request Apr 13, 2015
stephanebachelier added a commit to stephanebachelier/grunt-usemin that referenced this pull request Apr 13, 2015
stephanebachelier added a commit to stephanebachelier/grunt-usemin that referenced this pull request Apr 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants