Transformation addon does not respect resolveFrom option passed to app.import

[amiller-gh]

Problem

Tested in ember-cli 2.18.2 and 3.3.0

The AMD transform allows users to specify an optional resolveFrom. This allows addons to call app.import and reference their dependencies instead of resolving all node modules relative to the app.

// In file `/path/to/app/node_modules/addon-name/index.js`

// Resolves to `/path/to/app/node_modules/path/to/thing.js` 
app.import("node_modules/path/to/thing.js",{
  using: [{ transformation: 'cjs', as: 'asset-name' }],
});

// Resolves to `/path/to/app/node_modules/addon-name/node_modules/path/to/thing.js` 
app.import("node_modules/path/to/thing.js",{
  using: [{ transformation: 'cjs', as: 'asset-name' }],
  resolveFrom: __dirname,
});

The CJS transform attempts to resolve the file on disk here and here. However, the resolveFrom option is never passed – it always tries to resolve from parent.root.

This results in an error when the CJS transform can't find the module.

Potential Fixes

It doesn't appear that ember-cli passes the resolveFrom option to the custom transforms. If we want to enable this use case without changes to ember-cli, the option will need to be passed twice in userland:

// Duplicate options, but minimal changes and easier rollout
app.import("node_modules/path/to/thing.js",{
  using: [{ transformation: 'cjs', as: 'asset-name', resolveFrom: __dirname }],
  resolveFrom: __dirname,
});

Otherwise, the resolveFrom option can be passed along to custom transforms' processOptions hook to be made available for use: https://github.com/ember-cli/ember-cli/blob/v3.0.x/lib/broccoli/ember-app.js#L1711

Let me know if I'm off base here! Happy to help make these changes if you'd like to move forward with a fix.

[rwjblue]

In general, addons should avoid app.import and should instead use this.import which should automatically resolve dependencies relative to the addon itself.

I dont know that I have tests for that though, definitely happy to confirm/deny. Also, it still probably does make sense to support resolveFrom just to stay consistent...

[rwjblue]

ember-cli-cjs-transform/tests/transform-test.js

Lines 292 to 294 in 1c04d66

	it(
	'finds node_modules using the same algorithm as `require`',
	co.wrap(function*(assert) {

This test seems related?

[amiller-gh]

Could've sworn that I tried this.import as well – I thought the same thing seeing it try to resolve from parent.root, but parent always seemed to be the app, not the addon. Will confirm 👍