Skip to content

Change addon exported files from .coffee to .js#136

Merged
kimroen merged 2 commits intokimroen:masterfrom
mriska:addon-files
Feb 15, 2017
Merged

Change addon exported files from .coffee to .js#136
kimroen merged 2 commits intokimroen:masterfrom
mriska:addon-files

Conversation

@mriska
Copy link
Contributor

@mriska mriska commented Feb 7, 2017

This PR changes the *-addon blueprints to use javascript files instead of coffeescript files. Without this change, addons using ember-cli-coffeecript creates files that can't be consumed in a application not using ember-cli-coffeescript.

Fixes #131

@mriska
Copy link
Contributor Author

mriska commented Feb 8, 2017

Same here, will fix to make tests pass.

@mriska
Copy link
Contributor Author

mriska commented Feb 8, 2017

@kimroen, pushed updated code with amended tests. Travis builds green now.

@mriska
Copy link
Contributor Author

mriska commented Feb 13, 2017

@kimroen, What do you think of this? Is it merge-worthy?

Copy link
Owner

@kimroen kimroen left a comment

Choose a reason for hiding this comment

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

Great, although in addition to the comment about the expectCoffee lines, I have a thing I would like someone to investigate. It could be a separate issue, though.

These files are now more or less identical to what the default ember -addon blueprints do. Can we remove them from ember-cli-coffeescript and still have them work?

I don't think we have tests for that, so we need to add a test for the addon-version of all the components that have addon-variants. Is that something you have the time to look into?

.to.contain("export { default } from 'my-app/addon-imports/foo'");
.to.contain("export { default } from 'my-app/addon-imports/foo';");

expectCoffee(importFile);
Copy link
Owner

Choose a reason for hiding this comment

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

These lines expect that the contents of the file in question is valid CoffeeScript, and that is not relevant for these tests that only involve JavaScript, so it should be removed.

@mriska
Copy link
Contributor Author

mriska commented Feb 13, 2017

@kimroen the incorrect expectCoffee is now removed from the tests. I would suggest merging this now - I have created a new issue #140 for investigating if the addon blueprints can be removed completely. I'll be happy to take a look at that one separately.

@kimroen
Copy link
Owner

kimroen commented Feb 15, 2017

Great, thank you again!

@kimroen kimroen merged commit bac3ebc into kimroen:master Feb 15, 2017
@perlun
Copy link
Contributor

perlun commented Feb 17, 2017

Could we get a v1.16.1 with this fix included @kimroen? Pretty please with sugar on top. 😇

@kimroen
Copy link
Owner

kimroen commented Feb 20, 2017

Could we get a v1.16.1 with this fix included @kimroen? Pretty please with sugar on top. 😇

You absolutely could, but I'm in the middle of moving so I'm short on time. Hopefully soon!

kimroen added a commit that referenced this pull request Mar 26, 2017
@kimroen
Copy link
Owner

kimroen commented Mar 26, 2017

There we go! v1.16.1

@mriska mriska deleted the addon-files branch March 26, 2017 21:22
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.

3 participants