dfreeman / ember-cli-node-assets

Include stylesheets, images and other assets in your Ember app from node modules
MIT License
62 stars 6 forks source link

notes on merging with ember-cli-node-modules-to-vendor #3

Open kellyselden opened 8 years ago

kellyselden commented 8 years ago

dfreeman [7:43 AM]
Hi Kelly — a couple folks have called https://emberobserver.com/addons/ember-cli-node-modules-to-vendor to my attention this week asking about the difference between that and https://emberobserver.com/addons/ember-cli-node-assets

[7:44]
It looks like they’re setting out to solve some of the same problems—any interest in combining efforts?

[7:45]
I’m still assuming(/hoping) in the long run any addon in this space will ultimately be obsoleted by changes in the CLI itself :slightly_smiling_face:

kellyselden [9:37 AM]

obsoleted by changes in the CLI itself that’s the plan

[9:37]
I made it before someone pointed out your addon to me

[9:38]
Let me check your API

[9:39]
First difference I notice is you can’t import outside node_modules

[9:39]
not too big a deal

kellyselden [9:44 AM]
Ok, the only real difference is how we import

[9:46]
you have the slim array that wraps app.import I’m assuming, and I prefer the built in app.import to use all the options if necessary

app.import('vendor/a-file-from-the-folder-above.js', {
  using: [{ transformation: 'amd', as: 'some-package' }]
});

[9:46]
Does yours handle both modules and globals?

dfreeman [10:28 AM]

First difference I notice is you can’t import outside node_modules I was actually really curious about that — what was the use case you had in mind for that one? The original reason I built the addon was because I wanted to abstract away the module resolution dance for finding specific files, but I wasn’t thinking so much about things outside of node_modules (edited)

[10:29]

you have the slim array that wraps app.import I’m assuming, and I prefer the built in app.import to use all the options if necessary

[10:30]
the array doesn’t have to be flat strings, so the equivalent would be something like:

import: [{
  path: 'some/file.js',
  using: [{ transformation: 'amd', as: 'some-package' }]
}]

[10:30]
the path bit is a kludge for sure, but it basically just passes through any other keys to app.import (edited)

kellyselden [10:52 AM]
what do you think of import being optional so you can do it the old fashioned way if you want?

dfreeman [10:53 AM]
totally open to that

[10:54]
I pretty much started from my personal-taste best case API for the simple cases and worked backward from there as other needs came up

[10:54]
but having the ability to call app.import where/when you want as an escape hatch (or just preference) seems like a good call

kellyselden [11:00 AM]
as for outside node_modules, no use case, it’s just possible so I noted it in the readme

[11:00]
so your addon smart resolves node module package location between npm 2/3?

dfreeman [11:03 AM]
yep

[11:04]
basically if you could require() something in the target node module from your app/addon’s ember-cli-build.js/index.js/etc, node-assets will find it in the right place (edited)

[11:04]
https://github.com/dfreeman/ember-cli-node-assets/blob/master/index.js#L133

kellyselden [11:08 AM]
that’s cool

[11:08]

Does yours handle both modules and globals?

dfreeman [11:08 AM]
Not CommonJS

[11:09](or ES6)

[11:09]
But it’ll take AMD via app.import‘s new using option

[11:10]
Not a replacement for Browserify/Rollup/etc, though—pretty much just a bunch of glue for app.import at this stage (edited)

kellyselden [11:25 AM]
what about globals, like https://www.npmjs.com/package/location-origin npm location-origin Polyfill for window.location.origin

[11:25]
where app.import is basically just appending

dfreeman [11:26 AM]
it doesn’t generate a module shim, if that’s what you’re asking

[11:26]
but it’ll happily pass off any old file you give it to app.import

kellyselden [11:27 AM]
I see, that’s good

dfreeman commented 8 years ago

Thanks for capturing this! The coming week or so promises to be a little hectic for me, but I'll try to set aside some time over the weekend to write up some thoughts on a path forward post them here to get feedback from you and anyone else who's interested.

dfreeman commented 7 years ago

So recapping the conversation above, it looks like the two main things node-modules-to-vendor has that node-assets doesn't are:

Things node-assets has that node-modules-to-vendor doesn't:

Both addons ultimately support app.importing arbitrary files from an npm package into an app; the key difference is just the form factor of the import.

Given node-assets's existing adoption, I'll float using that as a starting point. In addition to the public and import keys it accepts for a given package today, I'd propose accepting a third vendor key such that these two configurations would be essentially equivalent:

'some-package': {
  srcDir: 'dist',
  import: [
    { path: 'some-file.js', ...jsOptions },
    { path: 'some-file.css', ...cssOptions }
  ]
}
'some-package': {
  srcDir: 'dist',
  vendor: {
    include: ['some-file.js', 'some-file.css']
  }
}
// ...
app.import('vendor/some-package/some-file.js', jsOptions);
app.import('vendor/some-package/some-file.css', cssOptions);

Importing from outside a node package seems to me to be somewhat orthogonal to the rest of the concerns of both addons, but I imagine we can come up with a way to integrate that as well.

It might also be interesting to explore using something like Rollup to enable bringing in packages in formats other than globals or AMD (i.e. ES2015 or CommonJS) for cases where they don't need the heavier Node compatibility layer provided by Browserify, but that also may be an orthogonal concern.