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

[INTERNAL] Improve jQuery deprecation message #8399

Merged
merged 2 commits into from Feb 3, 2019

Conversation

simonihmig
Copy link
Contributor

  • As discussed with @rwjblue, this makes the existing deprecation message more specific, stating explicitly that the jQuery integration per se is deprecated and will be removed from Ember itself
  • added tests

Making it more specific that the jQuery integration has been deprecated and will be removed.
app = new EmberApp({
project,
});

Copy link
Contributor Author

@simonihmig simonihmig Feb 3, 2019

Choose a reason for hiding this comment

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

This change was required as app would be undefined otherwise. It accidentally worked before, as app was initialized and leaking from another test before.

@rwjblue rwjblue merged commit 0f496b0 into ember-cli:master Feb 3, 2019
@@ -489,7 +489,11 @@ class EmberApp {
if (optionFeatures && !optionFeatures.isFeatureEnabled('jquery-integration')) {
return;
}
this.project.ui.writeDeprecateLine('Ember will stop including jQuery by default in an upcoming version. If you wish keep using jQuery in your application explicitly add `@ember/jquery` to your package.json');
this.project.ui.writeDeprecateLine(
Copy link
Member

Choose a reason for hiding this comment

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

Forgive me for being out of the loop, but is it not possible to put this in the ember-source addon itself?

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 think you are right, and given that it's ember-source which is actually deprecating things (i.e. causing breaking changes in the future), it would make more sense to put it there. Especially that ember-source and ember-cli might not be used in lock step.

On the other hand, IIRC this was already added a while ago when we started to not include jQuery anymore (which historically ember-cli did, not ember-source!) when @ember/jquery is detected. So when we want to move that message to ember-source, we would probably have to check the ember-source version here, to not issue a deprecation message twice!?

A bit of shenanigans involved, but I can do it if we agree it's worth it!?

Choose a reason for hiding this comment

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

It is possible to print out where jquery is being included? We have a project with tons of dependencies and it would be a huge task to sift through all of those and figure out which one is using it.

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.

None yet

4 participants