Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Replace String.trimRight() with regex in order to support IE10+ #7231

Merged
merged 3 commits into from
Aug 5, 2014
Merged

Replace String.trimRight() with regex in order to support IE10+ #7231

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd2a2ae
Replace String.trimRight() with regex in order to support IE10+
humphd Mar 18, 2014
8b21f3b
Add utils/Compatibility.js for shims, with shims for String trimRight…
humphd Mar 19, 2014
b8b8ef1
Rework utils/Compatibility loading as per review
humphd Mar 21, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/brackets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
*/
define(function (require, exports, module) {
"use strict";


Copy link
Contributor

Choose a reason for hiding this comment

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

Only changes to brackets.js are whitespace changes, so these changes should be removed.

// Load compatibility shims--these need to load early, be careful moving this
require("utils/Compatibility");
Copy link
Member

Choose a reason for hiding this comment

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

Does Require make any guarantees about the order that "sibling" require() calls are loaded in? @dangoor do you have any idea?

If it's not guaranteed, I wonder if it's better to put this directly in src/main.js (cluttery as that may be) or even load it directly from a separate <script> tag before we boot up Require?

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a similar question + answers: http://stackoverflow.com/questions/11581611/load-files-in-specific-order-with-requirejs

Copy link
Member

Choose a reason for hiding this comment

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

The accepted answer there boils down to 'explicitly require Compatibility.js in every module that calls trimRight(),' which seems clumsy and more than a little fragile.

The second answer listed there -- basically, chaining async require() calls in main.js -- sounds promising though.

However, if there's any sort of guarantee that the code in this PR will work as-is, that's definitely preferable. I couldn't find any docs that said either way. Anyone have a reference or spec for RequireJS's loading order?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just found a note in the official doc: http://requirejs.org/docs/1.0/docs/api.html#order

Copy link
Member

Choose a reason for hiding this comment

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

I read that earlier, but it's actually very vague. It seems to only pertain to JS files that aren't wrapped in a define().

Normally RequireJS loads and evaluates scripts in an undetermined order

but then...

The order plugin ... is not needed for scripts that use define() to define modules

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should go straight to the source :-) @jrburke, does RequireJS make any guarantee about the order that sibling require() calls are resolved in? We couldn't find anything definitive in the docs. Thanks!

More detail, if needed... if module "main" does this:

define(function(require, exports, module) {
    require("A");
    require("B");
});

Is the code in the body of A's define() guaranteed to run before B's (assuming A doesn't require B internally)? I.e., are the dependencies resolved as a sort of depth-first traversal, or is the order more nondeterministic?

Copy link

Choose a reason for hiding this comment

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

@peterflynn the order is not guaranteed, for two reasons:

  • other modules could have asked for 'B' before 'main' is loaded or has its dependencies registered.
  • module factories are called once they are loaded (if all their dependencies are involved), and that load order is dependent on network load order, which is indeterminate. If 'A' needs 'C', but 'B' has no dependencies, it is likely 'B' will be defined before 'A'.


// Load dependent non-module scripts
require("widgets/bootstrap-dropdown");
require("widgets/bootstrap-modal");
Expand All @@ -57,7 +60,7 @@ define(function (require, exports, module) {
require("thirdparty/CodeMirror2/addon/mode/overlay");
require("thirdparty/CodeMirror2/addon/search/searchcursor");
require("thirdparty/CodeMirror2/keymap/sublime");

// Load dependent modules
var Global = require("utils/Global"),
AppInit = require("utils/AppInit"),
Expand Down
42 changes: 42 additions & 0 deletions src/utils/Compatibility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
*/


/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
/*global define */

/**
* Compatibility shims for running Brackets in various environments, browsers.
*/
define(function (require, exports, module) {
"use strict";

// [IE10] String.prototype missing trimRight() and trimLeft()
if (!String.prototype.trimRight) {
String.prototype.trimRight = function () { return this.replace(/\s+$/, ""); };
}
if (!String.prototype.trimLeft) {
String.prototype.trimLeft = function () { return this.replace(/^\s+/, ""); };
}

});