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

Update blueprints to import modules directly #4070

Merged
merged 2 commits into from
Jan 19, 2016

Conversation

HeroicEric
Copy link
Sponsor Member

TL;DR;

This does two things:

  • Adds more tests to the adapter & model blueprints
  • Updates blueprints to import modules directly rather than access them through DS

I originally added the extra tests to cover the existing behavior before I changed it. If you'd rather have those split out into a separate PR LMK.

Details

  • Imports JSONAPIAdapter module directly in blueprint
  • Imports JSONAPISerializer module directly in blueprint
  • Imports Transform module directly in blueprint
  • Imports attr module directly in model blueprints when needed
  • Imports belongsTo and hasMany modules directly in blueprints when needed

@HeroicEric HeroicEric changed the title Use modules in blueprints Update blueprints to import modules directly Jan 13, 2016
@pangratz
Copy link
Member

FYI, I have an somewhat accompanying PR #4023 open, which would make your adapter test adapter when is named "application" obsolete.

The failing tests should be fixed once a new release for ember-cli-blueprint-test-helpers is cut, as pointed out by @trabus here #4023 (comment).

This looks very good to me 👍 ! Nice change importing modules directly!

- Import JSONAPIAdapter module directly in blueprint
- Import JSONAPISerializer module directly in blueprint
- Import Transform module directly in blueprint
- Import attr module directly in model blueprints when needed
- Import belongsTo and hasMany modules directly in blueprints when needed
@HeroicEric
Copy link
Sponsor Member Author

@pangratz turns out, the tests were failing because of a typo importStatment !== importStatement 😄

Would you like me to remove the adapter when is named "application" test?

@pangratz
Copy link
Member

No, you can leave it in I guess. This PR will likely be merged soon, so I will update the test in #4023 once this is the case.

}

if (shouldImportBelongsTo && shouldImportHasMany) {
importStatements.push('import { belongsTo, hasMany } from \'ember-data/relationships\';');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope for this pr but I've been thinking it may be useful to add a path that allows users to import belongsTo, hasMany and attr all on the same line.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@HeroicEric
Copy link
Sponsor Member Author

This should wait until #4075 is resolved

@HeroicEric
Copy link
Sponsor Member Author

^^ was fixed by #4091

bmac added a commit that referenced this pull request Jan 19, 2016
Update blueprints to import modules directly
@bmac bmac merged commit 1be6c62 into emberjs:master Jan 19, 2016
@bmac
Copy link
Member

bmac commented Jan 19, 2016

Thanks @HeroicEric

@HeroicEric HeroicEric deleted the use-modules-in-blueprints branch January 19, 2016 02:55
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.

None yet

4 participants