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

Travis and yarn issue on Ember 2.12 only #148

Comments

@RobbieTheWagner
Copy link

I'm unsure what is special about Ember 2.12, but that case is consistently failing across all my addons while all the other cases pass.

https://travis-ci.org/shipshapecode/ember-contact-form/builds/275019161
https://travis-ci.org/shipshapecode/ember-math-helpers/builds/274758588
https://travis-ci.org/shipshapecode/ember-vivus/builds/274420286

@rwjblue
Copy link
Member

rwjblue commented Sep 13, 2017

The 2.12 scenario is the only one for a default addon that uses npm packages (all of the others are bower specific). Changes landed late last week that seem to have caused this regression, but I'm not 100% sure where the issue is coming from.

@rwjblue
Copy link
Member

rwjblue commented Sep 13, 2017

What we do know, is that ember-cli-dependency-checker is throwing an error (which fails the build) saying that the package.json says to use 2.12 but we are actually using 2.15 (or whatever the addon has as its default). I don't see any code in ember-cli-dependency-checker that is aware of yarn.lock so this makes me wonder if we are actually running npm install in the case where we detect yarn.lock and do not have useYarn: true set.

@RobbieTheWagner
Copy link
Author

@rwjblue yeah, spoke at length about yarn issues with @kategengler on slack, and I think we decided the ember-try blueprints need to follow the same conventions as ember-cli and automatically add useYarn: true under certain cases.

@kellyselden
Copy link
Member

I haven't been experiencing this. Is it because of Travis npm cache benefits, or does this only affect yarn users?

@RobbieTheWagner
Copy link
Author

@kellyselden this is only if you use yarn.

@kategengler
Copy link
Member

Pretty sure this is the opposite scenario of the problems we had with automatically using yarn (where npm install followed by yarn install meant 💥) and somewhat the same problem (with a different cause) as #141. yarn install followed by an npm install seems to clear the node_modules directory, at least with npm@5

I investigated how ember-cli handles yarn: It has a --yarn (default: undefined) option for commands where that is relevant, if undefined, it uses yarn based on the presence of yarn.lock, if true it forces yarn and if false it forces npm.

I think ember-try should be consistent with ember-cli in this case but ember-try has the gotcha that it will not work unless yarn/npm is consistent with what originally installed the dependencies for a project. This also means that the package manager (yarn vs npm) must be the same through all scenarios.

Can we enforce this with an error? Is there a way to tell whether yarn or npm was used to install node_modules? Should we leave it to the user to be consistent?

Longer term, if ember-try were able to be installed and run globally, then ember-try could take over running the initial install and control the entire process.

@alexdiliberto
Copy link

alexdiliberto commented Sep 17, 2017

Seeing the same issue starting on a build from 4 days ago on an addon here:

https://travis-ci.org/alexdiliberto/ember-transformicons/builds/274381413

@RobbieTheWagner
Copy link
Author

Are the blueprints for ember-try in ember-try itself or are the files generated by ember-cli? I'm happy to PR in a fix that checks for the yarn flags.

@kategengler
Copy link
Member

Per my plan above, I hope to remove the yarn flag.

juwara0 added a commit to juwara0/ember-frost-notifier that referenced this issue Oct 18, 2017
juwara0 added a commit to juwara0/ember-frost-info-bar that referenced this issue Oct 19, 2017
onechiporenko pushed a commit to onechiporenko/ember-models-table that referenced this issue Oct 30, 2017
@kellyselden
Copy link
Member

marxsk added a commit to marxsk/ember-dynamic-fields that referenced this issue Nov 9, 2017
@RobbieTheWagner
Copy link
Author

This is fixed, as useYarn is now correctly added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants