Skip to content
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

Merged
merged 4 commits into from
Jun 24, 2014
Merged

Conversation

abuiles
Copy link
Member

@abuiles abuiles commented Jun 20, 2014

Previously you would do something like

  this.import('vendor/ember-cli-shims/app-shims.js', {
    ember: ['default']
  });

Now you will have to pass an object with exports key:

  this.import('vendor/ember-cli-shims/app-shims.js', {
    exports: {
      ember: ['default']
    }
  });

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 with imports so we better address this now.

abuiles added a commit to abuiles/ember-cli-ember-data that referenced this pull request Jun 21, 2014
@stefanpenner
Copy link
Contributor

availableModules -> exports?

@stefanpenner
Copy link
Contributor

is there a way we can deprecate?

@rwjblue
Copy link
Member

rwjblue commented Jun 21, 2014

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.

@abuiles
Copy link
Member Author

abuiles commented Jun 21, 2014

@rjackson @stefanpenner Just added a deprecation warning with support for old modules.

@abuiles
Copy link
Member Author

abuiles commented Jun 22, 2014

@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).'));
Copy link
Member

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.

@abuiles
Copy link
Member Author

abuiles commented Jun 22, 2014

@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']
    }
  });
```
@abuiles
Copy link
Member Author

abuiles commented Jun 22, 2014

@rjackson rebased.

abuiles added a commit to abuiles/ember-cli-ember-data that referenced this pull request Jun 22, 2014
abuiles added a commit to abuiles/ember-cli-ic-ajax that referenced this pull request Jun 22, 2014
@rwjblue rwjblue added this to the 0.0.37 milestone Jun 24, 2014
rwjblue added a commit that referenced this pull request Jun 24, 2014
Allow to pass options to import.
@rwjblue rwjblue merged commit 5572382 into ember-cli:master Jun 24, 2014
@rwjblue rwjblue deleted the refactor-import branch June 24, 2014 13:56
@WMeldon
Copy link
Contributor

WMeldon commented Jun 24, 2014

@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.

@abuiles
Copy link
Member Author

abuiles commented Jun 24, 2014

@rjackson @WMeldon there is a PR in both repos already.

@abuiles
Copy link
Member Author

abuiles commented Jun 24, 2014

@WMeldon btw, just updated the description here to mention exports.

rwjblue added a commit to rwjblue/ember-cli-ic-ajax that referenced this pull request Jun 24, 2014
Changed to use new `exports` option introduced by
ember-cli/ember-cli#1121.
rwjblue added a commit to rwjblue/ember-cli that referenced this pull request Jun 24, 2014
abuiles added a commit that referenced this pull request Jun 24, 2014
Update to latest ember-cli-ic-ajax to accomodate #1121.
@gf3
Copy link

gf3 commented Jun 26, 2014

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' });

@Panman82
Copy link
Contributor

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.

@pixelhandler
Copy link
Contributor

Also wondering how to use import with development/production

app.import({
  development: 'vendor/ember-canary/ember.js',
  production:  'vendor/ember/ember.prod.js'
});

@gf3
Copy link

gf3 commented Jul 2, 2014

@stefanpenner ping

@rwjblue
Copy link
Member

rwjblue commented Jul 2, 2014

@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).

flecno added a commit to flecno/ember-cli that referenced this pull request Jul 4, 2014
update code example according to ember-cli#1121
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants