Skip to content

Commit bfd684d

Browse files
committed
Fix pat-upload lazification for imperative callers
The split left pat-structure, pat-relateditems and the contentbrowser app importing the thin upload.js registration module and doing `new Upload()`. That module grafts the implementation asynchronously in init(), so the instance was missing its body (setPath, currentPath, the Dropzone wiring) right after construction, breaking those callers. Make upload--implementation.js a constructable Base.extend pattern again and point the imperative callers at it, so `new Upload()` yields a fully formed instance synchronously. The thin upload.js keeps registering the .pat-upload trigger and grafts the implementation prototype on first match, so the body still stays out of the eager patterns chunk (the implementation is only reached via lazy chunks: the structure app view, the contentbrowser component and the relateditems dynamic import). Registry.register is first-wins, so the eagerly imported thin module remains the registered .pat-upload pattern. Add a regression test asserting the implementation is constructable and exposes its methods synchronously.
1 parent 86f0cf3 commit bfd684d

6 files changed

Lines changed: 47 additions & 14 deletions

File tree

src/pat/contentbrowser/src/ContentBrowser.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import { fly } from "svelte/transition";
66
import _t from "../../../core/i18n-wrapper";
77
import { ensureIntlSupport } from "../../../core/intl-loader";
8-
import Upload from "../../upload/upload";
8+
import Upload from "../../upload/upload--implementation";
99
import contentStore from "./ContentStore";
1010
import {
1111
clickOutside,

src/pat/relateditems/relateditems.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ export default Base.extend({
402402
},
403403

404404
async initUploadView(disabled) {
405-
let Upload = await import("../upload/upload");
405+
let Upload = await import("../upload/upload--implementation");
406406
Upload = Upload.default;
407407

408408
const upload_button = this.$toolbar[0].querySelector(".upload button");

src/pat/structure/js/views/upload.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import $ from "jquery";
22
import _ from "underscore";
33
import PopoverView from "../../../../core/ui/views/popover";
4-
import Upload from "../../../upload/upload";
4+
import Upload from "../../../upload/upload--implementation";
55

66
export default PopoverView.extend({
77
className: "popover upload",

src/pat/upload/upload--implementation.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,24 @@
11
import $ from "jquery";
22
import _ from "underscore";
33
import _t from "../../core/i18n-wrapper";
4+
import Base from "@patternslib/patternslib/src/core/base";
45
import logger from "@patternslib/patternslib/src/core/logging";
56
import utils from "../../core/utils";
67

78
const log = logger.getLogger("pat-upload");
89

910
let Dropzone;
1011

11-
// Pattern config object — grafted onto the thin registered pattern in
12-
// upload.js and run there. Kept out of the eager patterns chunk.
13-
export default {
12+
// The full pat-upload implementation. It stays out of the eager patterns chunk:
13+
// the thin upload.js registration module loads it on first `.pat-upload` match
14+
// and grafts its prototype. Imperative callers (pat-structure, pat-relateditems,
15+
// the contentbrowser app) import this module directly and `new Upload(...)` it,
16+
// so it must be a constructable Base.extend pattern, not a plain object grafted
17+
// asynchronously.
18+
export default Base.extend({
19+
name: "upload",
20+
trigger: ".pat-upload",
21+
parser: "mockup",
1422
defaults: {
1523
showTitle: true,
1624
url: null, // XXX MUST provide url to submit to OR be in a form
@@ -497,4 +505,4 @@ export default {
497505
});
498506
return ri;
499507
},
500-
};
508+
});
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import $ from "jquery";
2+
import Upload from "./upload--implementation";
3+
4+
// pat-structure, pat-relateditems and the contentbrowser app construct upload
5+
// imperatively (`new Upload(el, options)`) and use the instance right away.
6+
// The implementation must therefore be a constructable pattern that exposes its
7+
// methods synchronously — not a config object grafted asynchronously by the thin
8+
// upload.js registration module.
9+
describe("upload implementation", function () {
10+
let $el;
11+
beforeEach(function () {
12+
$el = $('<div class="pat-upload"/>').appendTo("body");
13+
});
14+
afterEach(function () {
15+
$el.remove();
16+
});
17+
18+
it("is constructable and exposes its methods synchronously", function () {
19+
const upload = new Upload($el, { url: "/upload", currentPath: "/a" });
20+
expect(typeof upload.setPath).toBe("function");
21+
expect(typeof upload.getUrl).toBe("function");
22+
});
23+
});

src/pat/upload/upload.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,16 @@ export default Base.extend({
1010
parser: "mockup",
1111

1212
init: async function () {
13-
const impl = (await import("./upload--implementation")).default;
13+
// The implementation is a full Base.extend pattern (imperative callers
14+
// do `new Upload(...)`), so its instance behaviour lives on the
15+
// prototype.
16+
const proto = (await import("./upload--implementation")).default.prototype;
1417
// Defaults live with the implementation; merge them under the parsed
1518
// options (which must win), reproducing the eager pattern's options.
16-
this.options = $.extend(true, {}, impl.defaults, this.options);
17-
// Graft onto this single instance so consumers that instantiate Upload
18-
// directly (pat-structure, pat-relateditems, the contentbrowser app)
19-
// keep working against one object.
20-
$.extend(this, impl);
21-
return impl.init.apply(this, arguments);
19+
this.options = $.extend(true, {}, proto.defaults, this.options);
20+
// Graft onto this single instance so the pattern registers and behaves
21+
// exactly as before, just with the heavy body loaded on demand.
22+
$.extend(this, proto);
23+
return proto.init.apply(this, arguments);
2224
},
2325
});

0 commit comments

Comments
 (0)