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

Properly detect host ember-cli version. #190

Merged
merged 2 commits into from Nov 16, 2017

Conversation

Projects
None yet
2 participants
@rwjblue
Copy link
Member

rwjblue commented Nov 15, 2017

When running in a linked environment, the standard require.resovle technique to determine the currently running ember-cli version does not work properly (because the linked addon's ember-cli dev dependency will be found instead of the actual running ember-cli from the project).

The most common symptom of this issue is that modules are "double transpiled" (e.g. you have an AMD wrapped module inside of another AMD wrapper).

rwjblue added some commits Nov 15, 2017

Properly detect host ember-cli version.
When running in a linked environment, the standard `require.resovle`
technique to determine the currently running ember-cli version does not
work properly (because the linked addon's ember-cli dev dependency will
be found instead of the _actual_ running ember-cli from the project).

The most common symptom of this issue is that modules are "double
transpiled" (e.g. you have an AMD wrapped module _inside_ of another AMD
wrapper).
@Turbo87

This comment has been minimized.

Copy link
Contributor

Turbo87 commented Nov 15, 2017

shouldn't we fix this in ember-cli-version-checker instead?

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Nov 15, 2017

The issue is that it depends. If you are trying to detect the currently running processes version of ember-cli you do the thing I’m doing here, but Ember-cli-version-checker is mostly just checking what is installed within the specific root. In other words, checking the running processes version of ember-cli isn’t always correct...

@Turbo87

This comment has been minimized.

Copy link
Contributor

Turbo87 commented Nov 15, 2017

In other words, checking the running processes version of ember-cli isn’t always correct...

please elaborate 🤔

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Nov 15, 2017

It is correct in this case because the running processes Ember-cli will be controlling any build stuffs, however you could easily be running ember-cli-version-checker from within a process started (for example) with the globally installed ember-cli. In that scenario you want to detect the installed version, not the running one....

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Nov 15, 2017

I’m also not sure that I like the idea of special casing specific packages in the version checker (yes I know we made a special method for Ember itself).

IMHO, the majority of the time, what it does now is correct, and apecial casing sucks (generally speaking).

I’m happy to move this to the version checker if you feel very strongly. I just want to unblock folks currently being trolled by the bug I’m fixing.

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Nov 16, 2017

Discussed with @Turbo87 in slack, we are both 👍 on fixing the user facing issue here first and will circle around later to decide if ember-cli-version-checker should be updated/tweaked in some way...

@rwjblue rwjblue merged commit cad6a3f into babel:master Nov 16, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rwjblue rwjblue deleted the rwjblue:fix-cli-version-detection branch Nov 16, 2017

@rwjblue rwjblue added the bug label Nov 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment