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

EngineAddon -- unusual options config #11

Open XaserAcheron opened 7 years ago

XaserAcheron commented 7 years ago

Something @dfreeman and I discovered on Slack that I'm noting down for reference: ember-engines's EngineAddon class does some funky stuff that makes the documented way of defining options.nodeAssets not work.

A relevant quote:

dfreeman [2:05 PM] so if you have module.exports = { foo: 'bar' }; in an addon, the addon instance itself will have addon.foo === 'bar' but if you have module.exports = EngineAddon.extend({ foo: 'bar' });, the addon instance will have addon.options.foo === 'bar' so it ends up with double options

Effectively, the config is saved to module.exports.options.options.nodeAssets, which just don't work. ;)

Side-observation from the discussion: ember-cli-babel does its configuration in much the same way, so is it affected as well? Or does it skirt this somehow?

An extra thought: Should this be something we bring up to the ember-engines devs if we aren't the only ones affected? It may make more sense to treat it as a regression.

XaserAcheron commented 7 years ago

For anyone in the same boat: @dfreeman suggested a temporary workaround (which works great, BTW): take nodeAssets out of the options object for now.

Before:

const EngineAddon = require('ember-engines/lib/engine-addon');
module.exports = EngineAddon.extend({
    name: 'this-is-an-engine',
    options: {
        nodeAssets: {
            // ...
        }
    }
});

After:

const EngineAddon = require('ember-engines/lib/engine-addon');
module.exports = EngineAddon.extend({
    name: 'this-is-an-engine',
    nodeAssets: {
        // ...
    }
});

Just be prepared to change your code again once a fix has been made.

knownasilya commented 7 years ago

Is there a workaround or a fix for this coming in the near future?

I'm not having any luck with ember-rollup in engines, see https://github.com/ember-engines/ember-engines/issues/400#issuecomment-314532885

knownasilya commented 7 years ago

Oh, missed the comment above..

dfreeman commented 7 years ago

@knownasilya You could probably use this addon + broccoli-rollup to get things working, though ember-rollup gives a nicer form factor.

Ultimately I think https://github.com/ember-cli/rfcs/pull/108 plus the support for importing from node_modules in CLI 2.15 will make all this much better down the line.

knownasilya commented 7 years ago

@XaserAcheron do you have a full example of using node-assets in engines?

XaserAcheron commented 7 years ago

Not publicly-available, unfortunately. Perils of professional software development.

I'm not doing anything differently than in my second post above, though -- commenting out or removing the "options" wrapper object does the trick. Is there anything in particular that's acting strange for you?

knownasilya commented 7 years ago

Unfortunately it's this: https://github.com/ember-engines/ember-engines/issues/386

XaserAcheron commented 7 years ago

That looks like a problem related to manually including vendor shims in the tree. What does it have to do with ember-cli-node-assets, or this issue in particular?

[EDIT] Could it be you're asking for an ember-cli-node-assets sample to avoid the previously-mentioned issues? I keep asking the question "is there a bug with node-assets?" but I may be thinking backwards. Apologies if so.

knownasilya commented 7 years ago

@XaserAcheron node-assets puts things into vendor, but vendor doesn't seem to be handled properly in lazy engines.

XaserAcheron commented 7 years ago

Ah, okay -- thanks for the clarification. I think a new issue ought to be opened for that -- this one's a configuration quirk rather than a build issue of some sort.

I'll see if I can set aside a time to come up with a sample, since for whatever reason, it's Working For Me(tm). No guarantees though -- it's been a hectic couple of months so time is scarce. :(