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

Simplified exports in blueprints #137

Merged
merged 1 commit into from
Feb 13, 2017
Merged

Conversation

mriska
Copy link
Contributor

@mriska mriska commented Feb 8, 2017

This PR changes the blueprints from using a named variable to doing direct exports of the code. In addition to being more easy on the eyes, it fixes a problem with the compiled javascript file containing an undeclared variable in newer versions of the CoffeeScript compiler.

Fixes #135
Fixes #134

I have tested to create all of the blueprints changed:

ember g adapter foo-foo
ember g component foo-foo
ember g controller foo-foo
ember g helper foo-foo
ember g mixin foo-foo
ember g model foo-foo
ember g route foo-foo
ember g serializer foo-foo
ember g service foo-foo
ember g transform foo-foo
ember g util foo-foo
ember g view foo-foo

And the tests pass with the exception of view which I assume is due to the fact that a fairly recent Ember version is used which no longer supports views. I guess this blueprint can be removed at some point unless we want to keep it for people using it with older Ember version. On the other hand a case can be made for not helping them to create code that is deprecated in newer versions of Ember.

@mriska
Copy link
Contributor Author

mriska commented Feb 8, 2017

I didn't know there were node tests in here as well, I'll get those fixed.

@mriska
Copy link
Contributor Author

mriska commented Feb 8, 2017

@kimroen pushed updated code with amended tests. There is only one failure (beta) in Travis right now that looks like it needs poking to be rerun.

@mriska
Copy link
Contributor Author

mriska commented Feb 13, 2017

@kimroen, All builds are green now, do you think it should be merged, or are you still missing something?

@@ -1,4 +1,2 @@
<%= camelizedModuleName %> = () ->
export default () ->
Copy link
Owner

Choose a reason for hiding this comment

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

In the original blueprint this is a named function, but I haven't found a way to export a named function with the CoffeeScript syntax, so I'm not sure if it can be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kimroen I think you can do that with

export <%= camelizedModuleName %> () -> 

Like we have done here: https://github.com/kimroen/ember-cli-coffeescript/pull/137/files#diff-f9c146914b83a86c6a8d484dc1d877d6R4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do some testing and create a PR for that.

@kimroen
Copy link
Owner

kimroen commented Feb 13, 2017

This looks good - thanks again! 🎉

@kimroen kimroen merged commit 63caba4 into kimroen:master Feb 13, 2017
@mriska mriska deleted the simpler-exports branch February 13, 2017 12:06
@kimroen
Copy link
Owner

kimroen commented Feb 13, 2017

I noticed now that this missed some files, like the initializers.

@mriska
Copy link
Contributor Author

mriska commented Feb 13, 2017

@kimroen I'll check those out later today/tonight

@mriska
Copy link
Contributor Author

mriska commented Feb 13, 2017

@kimroen Created a new PR #139 that fixes initializer blueprint.

kimroen added a commit that referenced this pull request Mar 26, 2017
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.

Generated code no longer pass the tests Export classes directly instead of saving to variable
2 participants