Globalize jquery enhanced #623

marcoscaceres · 2016-03-09T07:45:36Z

See commit list... I'm going to break this up into several PRs where I can.

… deps

…s noisy

marcoscaceres · 2016-03-09T07:47:06Z

js/core/jquery-enhanced.js

@@ -0,0 +1,173 @@
+define(["jquery","core/utils"], function($, utils) {


This is a straight copy and paste from core/utils (where this code used to live). This code does not require review.

marcoscaceres · 2016-03-09T07:47:39Z

Actually, I'm just going to annotate the code. As it's not as many changes as it looks.

marcoscaceres · 2016-03-09T07:48:25Z

js/core/utils.js

-
-
+    [],
+    function () {


This is where I ripped out the additions to JQuery from.

marcoscaceres · 2016-03-09T07:51:54Z

Ok, finished annotations... ready for review.

halindrome · 2016-03-09T14:49:16Z

I am reviewing this. Trying to be careful though.

marcoscaceres · 2016-03-09T15:04:49Z

@halindrome really appreciate your time, thoroughness, help, and support! Your ideas are vastly improving what we are doing here.

halindrome · 2016-03-09T16:11:28Z

Question - do you think this version fixes the general problem that our utils.js methods are not exposed to jquery extended objects when they are used in extensions? 'Cause my testing shows that getDfnTitles, for example, is not a method on a jquery extended object in a JS file loaded by a respec-enabled page.

marcoscaceres · 2016-03-09T16:16:30Z

@halindrome, it should fix it. It literally just puts the extended jQuery into the window object before any of the plugins run.

marcoscaceres · 2016-03-09T16:17:01Z

js/core/jquery-enhanced.js

+    getTextNodes(this[0]);
+    return textNodes;
+  };
+  window.$ = $;


@halindrome here be the magic!!!!! 🌟

marcoscaceres · 2016-03-09T16:17:34Z

@halindrome, see: https://github.com/w3c/respec/pull/623/files#diff-2d019cf669f32febfab70c89006e39caR171

marcoscaceres · 2016-03-09T16:19:10Z

An extension can also define(["jquery-enhanced"]) ... I even think that is they ask for just "jquery", they will also get the enhanced, as the enhanced one is just enhancing the jQuery singleton.

halindrome · 2016-03-09T16:29:46Z

I think we are talking past one another. Before we started messing with this, tne utils.js module pushed a bunch of methods into the jquery function prototype so that they were all added to any jquery objects that are later created. So, when data-oninclude is defined that method will be called and jquery already has the extensions that permit easy integration with respec. That is not currently happening in this branch.

Extension scripts don't use require, and in any event there are tons of them out there. We can't ask them to all go change something in order to start working again.

halindrome · 2016-03-09T16:33:47Z

Just to be clear - these extensions don't ASK for anything. They are just snippets of JS that get run within the context of ReSpec because it just works.

marcoscaceres · 2016-03-09T17:04:04Z

Just to be clear - these extensions don't ASK for anything. They are just snippets of JS that get run within the context of ReSpec because it just works.

Understood. They don't need to ask. It will just work* (terms and conditions apply).

so long as they at least wait for isRespecReady or use the respecEvents thing.

halindrome · 2016-03-09T17:12:06Z

Well - in the case where I am testing my method is called from the data-oninclude handler and the methods are not htere. Check out http://dev.spec-ops.io/webpayments-ig/VCTF/use-cases - it loads from this branch's build of respec.

marcoscaceres · 2016-03-09T17:29:06Z

@halindrome, that version of ReSpec seems to be wrong (or didn't build?). It's missing this code. See, in particular, "profile-w3c-common.js". The code from this PR is not there?

halindrome · 2016-03-09T17:30:28Z

yikes..... weird. testing

marcoscaceres · 2016-03-09T17:30:34Z

@halindrome, in particular:

deps: ["core/jquery-enhanced", "Promise"]

is not there. And neither is the core/jquery-enhanced.js file.

marcoscaceres · 2016-03-09T17:31:27Z

In other news! YAY! the sourcemap work just came in handy... in even other news, it's 4:30 am, of shit I better got to sleep! 👍

marcoscaceres · 2016-03-09T17:31:34Z

*oh

marcoscaceres · 2016-03-09T17:37:00Z

Just to show it works for me locally (everything is there):

Did you do? ./tools/build-w3c-common.js 💤 😴

halindrome · 2016-03-09T17:38:32Z

Okay - no idea why it had not built when I changed branches. I do note that it is complaining about trying to load node_modules/promise-polyfill/Promise.js from the built version. I will try adding that to collection of required components that need to be included in the build. But yes, I can confirm that the object is extended!

Get some sleep.

marcoscaceres · 2016-03-09T17:40:51Z

I do note that it is complaining about trying to load node_modules/promise-polyfill/Promise.js from the built version.

Can you please post a new bug with the error? That should only happen if it's missing from your node_modules dir, or something is not right in the builder. I'm not seeing that error locally here.

halindrome · 2016-03-09T17:41:38Z

I will do. I do see that there is a dependency on it in that core/jquer-enhanced requires Promise, but we never load hqyer-enhanced in the profile-w3c-common.js file

halindrome · 2016-03-09T17:42:00Z

Oh - and otherwise LGTM.

marcoscaceres · 2016-03-09T17:51:02Z

Confirmed... it is going to the network for Promise.js. My thought is to just remove it, as all the browsers we target support it: http://caniuse.com/#search=Promise

halindrome · 2016-03-09T17:53:19Z

Huh. Remember people use this for editors drafts and we have no control at all who revirews those. Including people who use older user agents. ReSpec - it's not just for development any more. (tm)

marcoscaceres · 2016-03-09T18:01:41Z

@halindrome, sure, but we have to cut off somewhere. Promises have been natively supported in browsers for over a year, and all browsers are now evergreen. It seems highly unlikely people reading specs (specially ed drafts) won't be running a browser that is at most 3 months old.

halindrome · 2016-03-09T18:02:37Z

I am willing to try this - I just note for the record that it could bite us in the ass. In particular with Windows users who have not moved to Windows 10 and for whatever reason use IE.

marcoscaceres · 2016-03-09T18:05:51Z

Let's see what happens. If it's bad, we can quickly add the poly back.

halindrome · 2016-03-09T18:05:53Z

Assuming this passes I am comfortable with you merging and pushing a new release.

marcoscaceres · 2016-03-09T18:07:17Z

Ha! I think ./tests/headless.js just ruined the party.

halindrome · 2016-03-09T18:09:02Z

you're right. I had this problem the other day too. UGGH

marcoscaceres · 2016-03-09T18:13:12Z

Backed out the last commit.

marcoscaceres · 2016-03-09T18:14:38Z

Ok, will merge this after it turns green and figure out why Promise is not being inserted tomorrow (unless you want to have a look). The respecIsReady promise is also ready to merge, I hope.

halindrome · 2016-03-09T18:18:15Z

works for me. I will play with it a little in a new (local) branch once it is all merged.

Globalize jquery enhanced

marcoscaceres added 11 commits March 9, 2016 18:30

Feat (profile-w3c-common.js): Make jquery(-enhanced), Promise, global…

62cd352

… deps

Fix (js/{core|js|ui}): Use global jquery-enhanced

d4a9831

Feat (jquery-enhanced.js, utils.js) Make jQuery extras its own module

6d17536

Feat (jquery-enhanced-spec.js): Added tests for jquery enhanced

8ab7cd1

Fix (ui.js): Requires early binding to jquery

3eca755

Fix (tests): Make test behave better when loaded async

65f688f

Fix (SpecRunner.html): Allow jQuery to load, before we run the tests.

ab567f0

Fix (test-main.js): improve how tests and dependencies are loaded

378d644

Fix (SpecHelper.js): few additional defaults to conf, makes tests les…

751e91b

…s noisy

Feat (testFiles.json): Add jquery-enhanced-spec.js to list of tests

6eef6a7

Feat (SpecRunner.html): Make use of fetch + testFiles.json

71e209f

marcoscaceres reviewed Mar 9, 2016
View reviewed changes

js/core/utils.js

[],

function () {

Copy link

Member Author

marcoscaceres Mar 9, 2016

Choose a reason for hiding this comment

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

This is where I ripped out the additions to JQuery from.

This was referenced Mar 9, 2016

Fix (best-practices-spec.js): Unessesary use of jQuery #620

Closed

Make jquery-enhanced get used in right places places #598

Closed

Use fetch instead of ajax #624

Merged

marcoscaceres reviewed Mar 9, 2016
View reviewed changes

marcoscaceres force-pushed the globalize_jquery_enhanced branch from 678d29b to 71e209f Compare March 9, 2016 18:12

marcoscaceres pushed a commit that referenced this pull request Mar 9, 2016

Merge pull request #623 from w3c/globalize_jquery_enhanced

da7b6be

Globalize jquery enhanced

marcoscaceres merged commit da7b6be into develop Mar 9, 2016

marcoscaceres deleted the globalize_jquery_enhanced branch March 9, 2016 18:20

		@@ -0,0 +1,173 @@
		define(["jquery","core/utils"], function($, utils) {

Globalize jquery enhanced #623

Globalize jquery enhanced #623

Conversation

marcoscaceres commented Mar 9, 2016

marcoscaceres Mar 9, 2016

Choose a reason for hiding this comment

marcoscaceres commented Mar 9, 2016

marcoscaceres Mar 9, 2016

Choose a reason for hiding this comment

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

marcoscaceres Mar 9, 2016

Choose a reason for hiding this comment

marcoscaceres commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

marcoscaceres commented Mar 9, 2016

halindrome commented Mar 9, 2016