-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow to pass options to import. #1121
Conversation
availableModules -> exports? |
is there a way we can deprecate? |
Yes, I am very concerned with such a breaking change. Seems simple to check for the lack of a known prop (availableModules doesn't exist but the second param is an object) and deprecate, but still function. Then we can change after one release with the deprecation. |
@rjackson @stefanpenner Just added a deprecation warning with support for old modules. |
@rjackson ping |
var assetPath; | ||
|
||
if (typeof options === 'object' && typeof options.exports === 'undefined') { | ||
console.log(chalk.red('app.import - Passing modules object is deprecated. Please pass an option object with modules as export key (see http://git.io/H1GsPw for more info).')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also log the asset? Now that addons can add things via app.import
it may not be obvious where this deprecation warning is coming from.
@rjackson done, I'm rebasing now. |
Previously you would do something like ```js this.import('vendor/ember-cli-shims/app-shims.js', { ember: ['default'] }); ``` Now you will have to pass a key with availableModules: ```js this.import('vendor/ember-cli-shims/app-shims.js', { availableModules: { ember: ['default'] } }); ```
@rjackson rebased. |
Allow to pass options to import.
@rjackson Will need to update the ember-data and ic-ajax packages to this new syntax. And to clarify the syntax: this.import('vendor/ember-cli-shims/app-shims.js', {
exports: { //not availableModules
ember: ['default']
}
}); Caught me up for a second. |
@WMeldon btw, just updated the description here to mention exports. |
Changed to use new `exports` option introduced by ember-cli/ember-cli#1121.
Update to latest ember-cli-ic-ajax to accomodate #1121.
How does one import for various environments with the new syntax? Getting errors with the following for example: app.import({ development: 'vendor/lol/wat.js' }); |
As a suggestion, should a new issue be opened and a future milestone be targeted to remove the depreciation stuff? That way it doesn't get forgotten. |
Also wondering how to use import with development/production
|
@stefanpenner ping |
@gf3 - The syntax for multiple environments should not be changed. Also, this PR is closed, if you have a new issue to report please open one with a detailed bug report (including steps to reproduce and hopefully a demo repo showing the issue). |
update code example according to ember-cli#1121
Previously you would do something like
Now you will have to pass an object with exports key:
The intention behind is to make the
import
helper more extensible, initially this change was going to be part of #919 but it won't be ready between the next couple of days and I see more and more stuff going on withimports
so we better address this now.