Bundle codemirror into a single script

RESOLVED FIXED in Firefox 51

Status

defect
P1
normal
RESOLVED FIXED
3 years ago
11 months ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

unspecified
Firefox 51
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox51 fixed)

Details

(Whiteboard: [reserve-html])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
See https://bugzilla.mozilla.org/show_bug.cgi?id=1292592#c44.  Specifically:

"I think before landing that we'll need to also figure out a solution for bundling all of the codemirror scripts into a single script so we don't need to keep that big list in sync in 2 places.  I think the relative require paths inside the CM folder are not correct right now, but we could get that fixed"

Opening this bug to do that
(Assignee)

Updated

3 years ago
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Flags: qe-verify?
Whiteboard: [reserve-html]
Comment hidden (mozreview-request)

Comment 3

3 years ago
mozreview-review
Comment on attachment 8789937 [details]
Bug 1301790 - Bundle codemirror into a single script;

https://reviewboard.mozilla.org/r/77968/#review76468

::: devtools/client/jar.mn:34
(Diff revision 2)
> -    content/sourceeditor/codemirror/addon/fold/foldcode.js (sourceeditor/codemirror/addon/fold/foldcode.js)
> -    content/sourceeditor/codemirror/addon/fold/brace-fold.js (sourceeditor/codemirror/addon/fold/brace-fold.js)
> -    content/sourceeditor/codemirror/addon/fold/comment-fold.js (sourceeditor/codemirror/addon/fold/comment-fold.js)
> -    content/sourceeditor/codemirror/addon/fold/xml-fold.js (sourceeditor/codemirror/addon/fold/xml-fold.js)
> -    content/sourceeditor/codemirror/addon/fold/foldgutter.js (sourceeditor/codemirror/addon/fold/foldgutter.js)
>      content/sourceeditor/codemirror/addon/hint/show-hint.js (sourceeditor/codemirror/addon/hint/show-hint.js)

Was there a particular reason for excluding tern.js and show-hint.js from the bundle?

Comment 4

3 years ago
mozreview-review
Comment on attachment 8789937 [details]
Bug 1301790 - Bundle codemirror into a single script;

https://reviewboard.mozilla.org/r/77968/#review76470
Attachment #8789937 - Flags: review?(gl) → review+
(Assignee)

Comment 5

3 years ago
(In reply to Gabriel Luong [:gl] (ΦωΦ) from comment #3)
> Comment on attachment 8789937 [details]
> Bug 1301790 - Bundle codemirror into a single script;
> 
> https://reviewboard.mozilla.org/r/77968/#review76468
> 
> ::: devtools/client/jar.mn:34
> (Diff revision 2)
> > -    content/sourceeditor/codemirror/addon/fold/foldcode.js (sourceeditor/codemirror/addon/fold/foldcode.js)
> > -    content/sourceeditor/codemirror/addon/fold/brace-fold.js (sourceeditor/codemirror/addon/fold/brace-fold.js)
> > -    content/sourceeditor/codemirror/addon/fold/comment-fold.js (sourceeditor/codemirror/addon/fold/comment-fold.js)
> > -    content/sourceeditor/codemirror/addon/fold/xml-fold.js (sourceeditor/codemirror/addon/fold/xml-fold.js)
> > -    content/sourceeditor/codemirror/addon/fold/foldgutter.js (sourceeditor/codemirror/addon/fold/foldgutter.js)
> >      content/sourceeditor/codemirror/addon/hint/show-hint.js (sourceeditor/codemirror/addon/hint/show-hint.js)
> 
> Was there a particular reason for excluding tern.js and show-hint.js from
> the bundle?

They are used directly by the autocomplete module: https://dxr.mozilla.org/mozilla-central/source/devtools/client/sourceeditor/autocomplete.js#12.  And TernServer wasn't getting added onto the CodeMirror object when they were bundled.  It's probably possible to get this to this work but makes sense as a follow up I think.
Comment hidden (mozreview-request)

Comment 8

3 years ago
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/8d2a514d0f23
Bundle codemirror into a single script;r=gl

Comment 9

3 years ago
mozreview-review
Comment on attachment 8789937 [details]
Bug 1301790 - Bundle codemirror into a single script;

https://reviewboard.mozilla.org/r/77968/#review76512

::: devtools/client/.babelrc:1
(Diff revision 3)
> + { "presets": [ "es2015" ] }

this could be done in package.json without add .babelrc, like 

```
babel: {
  "presets": [ "es2015" ]
}
```

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8d2a514d0f23
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Iteration: --- → 51.3 - Sep 19
Flags: qe-verify? → qe-verify-
Priority: P3 → P1

Updated

3 years ago
Depends on: 1304262

Updated

3 years ago
Blocks: 1304262
No longer depends on: 1304262

Updated

11 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.