duojs / duo

A next-generation package manager for the front-end
3.42k stars 118 forks source link

How will duo handle non-CSS images/assets? #171

Closed dominicbarnes closed 9 years ago

dominicbarnes commented 10 years ago

Suppose I'm using an <img> tag in a .html template, duo would never be made aware of such an asset. (and thus it would never be moved to build/) In component, this is where manifests came in handy, so I'm wondering how duo will work with such a case?

Will it just be up to the developer to have a script that copies known images to build/ after duo has run? Is there any way duo could be notified of these assets? (when does duo look at manifest files?)

cristiandouce commented 10 years ago

I was asking myself the same thing. It's for me the last thing to completely move from Component to Duo

dominicbarnes commented 10 years ago

@cristiandouce my thoughts exactly :)

ianstormtaylor commented 10 years ago

Is it only an .html issue now since it's handled for .css and .js already? or is it like a pandora's box of trying to catch all of the possible non css and js places?

Could always cop out and have --assets <dir> and .assets(dir) on the API? I wonder if it comes up only in app use cases or also shareable packages use cases.

dominicbarnes commented 10 years ago

I would prefer to avoid magic. I think just a way to inform duo of relevant assets (via a manually composed manifest/list) is probably the most reasonable way to approach this.

We could use some DOM inspection for HTML files. I would also like to support Handlebars or Jade templates in some way. If we have such logic in JS and CSS, I don't see why HTML wouldn't be a logical next step.

cristiandouce commented 10 years ago

Yup. I like the approach to specify assets paths on a manifest. IMO is the less noisy.

"assets": [ "./photos", "./icons" ],

Another option may be to apply some set of basic rules to resolve assets in files that match the string-to-js parser.

Or maybe let to just require('./some/dir'); and if no main file found inside it let Duo copy the whole directory as if it were all assets.

dominicbarnes commented 10 years ago

Maybe the next-best thing is to allow require("./my-image.png"); (JS) or @import "./my-image.png"; (CSS)

(of course this could be any type of asset, not just images)

matthewmueller commented 10 years ago

We could use some DOM inspection for HTML files. I would also like to support Handlebars or Jade templates in some way. If we have such logic in JS and CSS, I don't see why HTML wouldn't be a logical next step.

This was the original plan (a duo-html). I'm not sure there's a way without rewriting the paths though. If there is, that's the path I'd like to pursue. We could even specify entry files in HTML and then we can build CSS and JS from HTML. Howerver, if we need to rewrite paths, we probably need some middleware to serve it. I feel like that's starting to creep a bit.

matthewmueller commented 10 years ago

For now it might be best to place those files in a /public and serve them directly. Not ideal though.

lancejpollard commented 10 years ago

Another example of non-CSS/JS assets is webgl shaders. I'm sure there will be many other use cases that pop up as well of non-standard assets.

What was nice about component is you could handle all of these cases by just adding them to the manifest.

https://github.com/graphicsjs/primitive-shader/blob/master/component.json

If we wanted to avoid the manifest, then it seems that basically you would have to have a "resolver" for each type of asset, and then you would have to add them as plugins to duo:

But the problem with having this approach is.. Say you have your new repo and you add one of these components with non-standard assets as dependencies. You will have to know that they are non-standard, and so add the plugins to your current repo to be able to build them correctly. That's obviously not the way to do it haha. Because then it wouldn't "just work" without some extra leg work on your part to make sure you have the build all properly wired up.

Maybe it will work though if we just require('./non-standard/asset.xyz')? Does it already work like this? Basically the same as the component.json templates field? If so then that would handle at least the cases where you're requiring non-standard assets in JavaScript, but still doesn't resolve the issue with html attributes pointing to assets..

matthewmueller commented 10 years ago

Oh interesting, so it works in component by passing them through string-to-js? Duo bundles that support right in. We probably just need to update: https://github.com/component/duo-string-to-js/blob/master/index.js#L17 to include shaders and anything else.

But the problem with having this approach is.. Say you have your new repo and you add one of these components with non-standard assets as dependencies. You will have to know that they are non-standard, and so add the plugins to your current repo to be able to build them correctly. That's obviously not the way to do it haha. Because then it wouldn't "just work" without some extra leg work on your part to make sure you have the build all properly wired up.

Yah, this seems like a bit of a deal-breaker to me haha.

lancejpollard commented 10 years ago

Cool cool, yeah will be nice to be able to include arbitrary strings :)

What if we just relax https://github.com/component/duo-string-to-js/blob/master/index.js#L29 a bit so it will just default to pure string-to-js? Otherwise it's one of those things where we start having arbitrary reasons to not include support for some particular file type because one person has a strong opinion lol (even though they probably don't use that particular feature).

Basically, it's inevitable that someone will come along and say:

Hey, can you add xml to the rtype regexp please?

and someone else will respond:

Why would you use XML, stop using XML. No.

But there's no reason not to include XML support if it's this easy, the restriction is totally arbitrary.

So if we just removed that regexp then it would:

stephenmathieson commented 10 years ago

Wow. That's a really convincing argument. +1

lancejpollard commented 10 years ago

i think this adds support for arbitrary strings: https://github.com/component/duo-string-to-js/pull/2

matthewmueller commented 10 years ago

-1 from me, but we can talk about it more. For example:

require('image.jpg')

will load the entire image into the JS file. I feel like adding to the regexp isn't hard. And I don't think there's much to discuss about adding HTML, XML, etc, etc.

The other option is to have a blacklist instead of a whitelist.

matthewmueller commented 10 years ago

The other reason I say this is right now you can actually let an image pass through the transforms and it will get symlinked (at least i'm pretty sure, haha).

stephenmathieson commented 10 years ago

Ahh didn't think about huge files like images, etc

Then again, why would someone require() an image?

lancejpollard commented 10 years ago

Yeah I think we might be thinking two different things... In a JS file (which is what that PR is for), you always want a string except when it's JavaScript or JSON (which is valid javascript). So if its anything but JS, it should be a string. So if you wanted to require an image, you'd probably do a base64 string, so that would work with this PR.

yields commented 10 years ago

Hey, can you add xml to the rtype regexp please? and someone else will respond:

Why would you use XML, stop using XML. No.

hahaha great point! :D

dominicbarnes commented 10 years ago

Yeah, the more I think about it require("./image.png"); is weird unless you want it to become a base64 string that you could use with <canvas> or injected into the DOM as a data-uri.

For the same reason, I think doing this via @import "./image.png"; is not what I was really asking for either, but it has it's own use-case I imagine.

I'm just talking about informing the duo builder that I want some assets copied to build/. (preferably under some predictable hierarchy, or at least to the path relative to the root) I think using a top-level manifest would be fine (and allowing globs)

matthewmueller commented 10 years ago

Yah, you can definitely control this from happening. I guess I was thinking of keeping the plugins strict and as independent as possible so there's less chance of unexpected issues from plugins clashing with each other.

Yeah, the more I think about it require("./image.png"); is weird unless you want it to become a base64 string that you could use with or injected into the DOM as a data-uri.

I could probably see this being a thing, especially for 5x5 repeating backgrounds or something.

For the same reason, I think doing this via @import "./image.png"; is not what I was really asking for either, but it has it's own use-case I imagine.

Right now @import "..." and url(...) dependencies are treated the same. Though string-to-js doesn't apply to non-js entry files.

cristiandouce commented 10 years ago

So... I'm a little lost.

Let's say I have the following html:

<img src="some-asset.png">

How would duo resolve to copy that image as an asset? As an end user I would expect duo to resolve it for me... (or maybe not, if I had the chance to tell via component.json what assets to copy. See below...)

And a more complex example, let's say I have a template html file like this:

<img src="path/to-{changing}-img.png">

I was thinking that it could be handled by an assets key in the component.json relative to that view/template component/module.

{
  "assets": ["path/*.png"]
}

Is this covered right now? Or how should we resolve this kind of needs?

ismay commented 10 years ago

Maybe what's needed is a more global method of passing remote dependencies to Duojs? I'm finding that for my issues at least, that's the only method that I'd need in order to integrate Duojs with my other build tools.

For example, my .html building process is done by Assemble, which takes .hbs as entry points (of sorts), can already parse for local dependencies (and with a plugin hopefully recognise external dependencies) and build them. That part doesn't have to be duplicated by Duo. But what I can't do right now is connect Assemble to Duo to pass any dependencies Assemble might need to Duo (and have Duo fetch them).

It makes sense to outsource part of this process to external build tools, but keep all the dependency management in one place (Duo). Making the API more global, instead of specialised for each type of dependency might make it easier to manage as well?

Ref: https://github.com/assemble/assemble/issues/570

dominicbarnes commented 10 years ago

How about using a .gitignore-type file to tell duo about assets that need to be copied? For example:

client.assets

images/*
fonts/*

Then, passing this file into duo causes it to copy the files listed into the build directory. (using the same pattern as it would in any other CSS-included asset)

$ duo client.assets -t assets

It seems like the flow makes sense in duo, as it is an entry file that can be parsed and contains lists of local files that duo can incorporate into the build. The only difference is that this file wouldn't need to be written to the build/ dir at all (although it wouldn't be the end of the world if it made it in there)

This probably doesn't need to be in core, but what I do think belongs in core is the ability to set custom types. (it looks like that's the eventual intent, right?)

If a type has a way to inform duo about dependencies it finds, or about local files/assets it needs to copy/symlink, this would be all we need to accomplish this.

ismay commented 10 years ago

@dominicbarnes If the file would allow for remote dependencies as well then it would basically solve all my issues.

cristiandouce commented 10 years ago

What happened with the images, fonts and files keys from the component.json? Wouldn't those need to be supported for fallback compat?

Besides, what @dominicbarnes describes for the client.assests is the same way the key files behaves. It just copies (or symlinks) any file described in it. We may allow the forms images/* as well.

stephenmathieson commented 10 years ago

client.assets

-1 on a metadata file. it kind of goes against the whole point :p

What happened with the images, fonts and files keys from the component.json? Wouldn't those need to be supported for fallback compat?

  • fonts: chances are, your fonts are included via @font-face so duo will pick them up. if not, then what is their purpose?
  • images: background-image: url('./foo.png')
  • files: what's the point?
cristiandouce commented 10 years ago

@stephenmathieson how would you handle an asset image for an img tag in an html template?

<img src="/where/am/i.jpeg">

That's not a background image... and there is no way, as far as I can tell, to let Duo know that image should get copied along to the build folder I intend to server/reach from my app.

Does that make any sense?

cristiandouce commented 10 years ago

Althoug I get the whole point is to not depend on manifests for a component to exist. The edge cases like this could be handled by an optional helper manifest with special instructions for the build/copy/distribution process.

Not required, but yet supported.

That's my opinion though.

ismay commented 10 years ago

@stephenmathieson I agree that it would be best for Duo to work without a manifest, but if duo's going to go down that road, it'll need to be possible to use html as an entry point too. Which @MatthewMueller kind of argued against in #326 :

Right now Duo doesn't support using HTML files as entries (unless there's a transform that turns it into JS or CSS. I know we had plans to add HTML support in the past, but I'm not sure that it could be done cleanly, given that servers can serve pretty much whatever path they want for the <link>, <script>, <img> etc.

I'd say that it would make the most sense to allow for html as entry points (with a plugin or as core functionality). It might be complicated, but not including html will mean that you're going to miss a lot of dependencies (which people ITT want to solve with a manifest).

stephenmathieson commented 10 years ago

it'll need to be possible to use html as an entry point too

that'd be pretty awkward:

$ duo index.html > build.html

with a plugin

yep. good idea! how about something like (not tested):


var thunk = require('thunkify');
var cp = thunk(require('cp'));
var cheerio = require('cheerio');

duo.use(function* (file) {
  if ('html' != file.type) return;
  // magic:
  var assets = scanForAssets(file.src);
  var dir = file.duo._installTo;
  // copy all assets to the specified "build" dir:
  for (var i = 0, asset; asset = assets[i]; i++)
    yield cp(file.path, path.join(dir, file.id); 
});

function scanForAssets(html) {
  var $ = cheerio.load(html);
  // ...
}

and have your js require('./template.html') as you normally do

yields commented 10 years ago

agree with @stephenmathieson it should be a plugin, i recently wrote a plugin (with cheerio) to prefix html classes, duo makes it really easy.

stephenmathieson commented 10 years ago

;)

ismay commented 10 years ago

that'd be pretty awkward: $ duo index.html > build.html

I don't see why that would be awkward though. To me it's way more akward to require html (if it's not a template) from my js just so duo will process any dependencies that my html might have. If it's a template and it'll be used in html I can understand doing that, but require'ing big static chunks of html in my js doesn't make sense to me.

I can understand that you wouldn't want to build html with duo (even though I think that would be great, and that the actual build itself it could be handed off to assemble or metalsmith), but it would make sense to me to at least pass html through a duo pipeline so that any dependencies can be picked up.

stephenmathieson commented 10 years ago

why do you need a template if you don't require it? if it's a server-side thing, shouldn't your backend render it/handle its assets for you?

ianstormtaylor commented 10 years ago

Just a thing to think about: this seems like a 90% use case, so it should either be a plugin that is bundled in core, or in core itself. Most every real project will need these types of files: favicons, sitemaps, rss feeds, etc. all copied into build/ so you can just serve a single piece.

dominicbarnes commented 10 years ago

Yeah, I don't care whether or not this is included in core, my main desire here is for duo to allow me to do this in a plugin. Right now, it's not even possible, and that should be changed.

ismay commented 10 years ago

@stephenmathieson

It's not that I need a template. I'm using assemble (and sometimes metalsmith) to build a static site. Duo's also a part of that build process and at the moment there's no way to inform it of any assets that are only mentioned in the html.

Btw, whether this would be in core or a plugin is not important to me. Either would be fine by me.

matthewmueller commented 10 years ago

I'm +1 for adding a way to do this. I think it should be a plugin though but I'm still not sure it should be a part of core yet.

dominicbarnes commented 10 years ago

@MatthewMueller I'll open another issue at the file-deps repo, I think that lib should allow registering alternate types. And duo can just expose a way to hook into that registration API. (I think that's a good start, I'll start working on actual code before long and we can evaluate later)

dominicbarnes commented 9 years ago

In response to #400, I think it makes sense to allow duo to accept folders as arguments as well, where it will (recursively) copy or symlink just like any other asset. Of course, it will maintain the directory structure of the assets it copies, that way the dev can predict what the URL will ultimately be.

The next step after that would be to treat those other assets like any other entry file, including running plugins against them. (the duo.json cache could probably store things encoded with base64) This would open the door to so many awesome plugins, such as image compression/optimization, gzip compression, etc!

felixfbecker commented 9 years ago

Hi everyone, I just experimented with duo and wanted to share my usecase. I am writing a big Angular app where I don't want to bundle everything in one big file. I'm lazy-loading CSS with angular-css, so the CSS files should just be copied over to build with the same folder structure. I also use Angular's templateUrl loading, so I need to copy all templates over to build aswell. All I want to concat are the several JS files with Angular module definitions. I really like how duo works without any .json file etc, but I need a way to tell duo to copy these html and css files over to build. I also agree that it is simply weird to require() a html or css file in JS. Is there any progress on how duo is going to handle this?

dominicbarnes commented 9 years ago

@freshfelicio you aren't required to bundle everything into a single file with duo. Each file you pass as an "entry file" to duo will be copied to the build/ directory. In your case, simply pass each of your CSS files to duo. (of course, anything those files @import will be inlined)

As for HTML files, you can pass those to the CLI as well, and they'll just be copied to the build/ directory as well. (anything that's not JS or CSS is just copied)

Here's a trivial example of what you can do. This assumes you have multiple "pages" that have their own JS/CSS/HTML template. (this could likely apply to any other "bundling" strategy)

$ duo pages/*/index.js pages/*/index.css pages/*/template.html
felixfbecker commented 9 years ago

That's great, but I got a lot of files. I tried duo app/**/*.* but it doesn't work, it just spits out some empty lines to the command prompt. I really like duo, how it doesn't need any .json file and how it parses css files for fonts for example, but I just can't get it to work how I need it...

dominicbarnes commented 9 years ago

@freshfelicio try just duo app, it should recursively process any and all files in that directory. If you're still having trouble, let's create a new Github issue and iron this out.