Skip to content

Commit

Permalink
Recognizes jquery v1.12 for ember views
Browse files Browse the repository at this point in the history
  • Loading branch information
@jkole
jkole committed Jan 8, 2016
1 parent a364b39 commit bdd7784
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion packages/ember-views/lib/system/jquery.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ if (environment.hasDOM) {

assert(
'Ember Views require jQuery between 1.7 and 2.1',
jQuery && (jQuery().jquery.match(/^((1\.(7|8|9|10|11))|(2\.(0|1)))(\.\d+)?(pre|rc\d?)?/) || Ember.ENV.FORCE_JQUERY)
jQuery && (jQuery().jquery.match(/^((1\.(7|8|9|10|11|12))|(2\.(0|1)))(\.\d+)?(pre|rc\d?)?/) || Ember.ENV.FORCE_JQUERY)
);

if (jQuery) {
Expand Down

9 comments on commit bdd7784

@ryanhollister
Copy link

Choose a reason for hiding this comment

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

there's gotta be a better way to accomplish this. what happens when a new version drops?

@jkole
Copy link
Author

@jkole jkole commented on bdd7784 Jan 9, 2016

Choose a reason for hiding this comment

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

Definitely not ideal to have the dependency checking hard-coded into a regexp. I have all of an hour's experience with ember, so I'm not entirely sure what purpose the run-time dependency checking serves given that ember's build tools should be packaging the required versions anyway. But if this check is important, it would be great to have an actual semver check at run-time against whatever is in ember's bower.json...but that seems like ridiculous overkill...

Would an alternative be to change the jquery dependency declaration to match the regexp...something like >=1.7.0 <=1.12.0 || >=2.0.0 <=2.0.2 ? Not ideal, but at least it enforces some consistency between the declared dependency and the run-time check and reduces the risk that newcomers to the framework (like me) will be immediately stymied by an error whenever a new jquery version drops...

@ryanhollister
Copy link

Choose a reason for hiding this comment

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

I'd vote to remove this assert all together. leave dependency management to bower and and npm. If someone is still manually managing dependencies then they will need to read the docs.

@jkole
Copy link
Author

@jkole jkole commented on bdd7784 Jan 9, 2016

Choose a reason for hiding this comment

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

+1

@stefanpenner
Copy link
Member

Choose a reason for hiding this comment

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

assertion removed #12793, their will be a patch release to 1.13.x today.

@bichotll
Copy link

Choose a reason for hiding this comment

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

@stefanpenner can you also fix 1.12.x? It broke the code of the main app of the company I'm working for (not production tho). Really annoying. Now I have to fix and check out everything works properly.

Maybe I'm confused, but should you not create a new tag for every patch?
http://semver.org/

Tnx

@stefanpenner
Copy link
Member

Choose a reason for hiding this comment

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

@bichotll please lock your jquery to the version known to work. I don't believe we will release a non security patch to a version of ember that is that old.

I believe this has been resolved, as we have just removed the assertion

@johnnyshields
Copy link
Contributor

Choose a reason for hiding this comment

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

@stefanpenner please help by patching older versions of ember--at least 1.10, 1.11 and 1.12--so that they have a smoother upgrade path (i.e. jQuery can be upgraded independently of Ember). The effort required here is minimal and the benefit to legacy users is large.

@rwjblue
Copy link
Member

Choose a reason for hiding this comment

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

We have backported this to 1.13, 2.2, and it is included in 2.3. We do not intend to backport further, as there is no real need. It is pretty trivial to lock jQuery to 1.11 or 2.1, and there doesn't seem to be a major reason that I can see in the change logs that would force users to be required to upgrade jQuery further.

Please sign in to comment.