Skip to content

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

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

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

Conversation

@mriska

@mriska mriska commented Feb 7, 2017

Copy link
Copy Markdown
Contributor

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

mriska commented Feb 8, 2017

Copy link
Copy Markdown
Contributor Author

Same here, will fix to make tests pass.

@mriska

mriska commented Feb 8, 2017

Copy link
Copy Markdown
Contributor Author

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

@mriska

mriska commented Feb 13, 2017

Copy link
Copy Markdown
Contributor Author

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

@kimroen kimroen left a comment

Copy link
Copy Markdown
Owner

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
Copy Markdown
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

mriska commented Feb 13, 2017

Copy link
Copy Markdown
Contributor Author

@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

kimroen commented Feb 15, 2017

Copy link
Copy Markdown
Owner

Great, thank you again!

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

perlun commented Feb 17, 2017

Copy link
Copy Markdown
Contributor

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

@kimroen

kimroen commented Feb 20, 2017

Copy link
Copy Markdown
Owner

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

kimroen commented Mar 26, 2017

Copy link
Copy Markdown
Owner

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