Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lazily load shepherd dependency #304

Closed
st-h opened this issue Apr 28, 2019 · 6 comments · Fixed by #305
Closed

lazily load shepherd dependency #304

st-h opened this issue Apr 28, 2019 · 6 comments · Fixed by #305
Labels
enhancement

Comments

@st-h
Copy link
Contributor

st-h commented Apr 28, 2019

I was wondering if we could use ember-auto-import to allow lazy loading of the shepherd dependency? The shepherd dependency currently is the largest external one within my app, however might not be required at all.
I'll be happy to submit a PR, but wanted to check first if you would be interested in this
@RobbieTheWagner
Copy link
Owner

@st-h this sounds like a good idea to me. The main reason we are using ember-cli-node-assets, is so we can allow the consumer to specify a theme in the config and to wrap Shepherd in a fastboot transform, so it does not attempt to use window etc when in node. We'll need to account for these things when switching to ember-auto-import, but I do think it would be a good thing to do.

@st-h
Copy link
Contributor Author

st-h commented Apr 28, 2019

thanks for the input. I will give it a try, however I am currently not sure how to exactly deal with the templating/fastboot stuff.
Is ember-shepherd supposed to be included within FastBoot rendering? In my app I noticed that shepherd often caused issues with fastboot due to the autoscroll lock dependency which tries to attach event handlers. So far, I never inject the help service directly, but work around it using an initializer which does nothing in case of fastboot being active.
However, the most important question for now seems to be if fastboot should render shepherd as well, or just exclude it?

@RobbieTheWagner
Copy link
Owner

@st-h Shepherd is not included when running in fastboot and it should not be.

@st-h st-h mentioned this issue Apr 29, 2019
Merged
@st-h
Copy link
Contributor Author

st-h commented Apr 29, 2019

@rwwagner90 thanks. I'll see if I can find the reason for the exception I occasionally see as well.

just attaching the trace for reference here:

TypeError: window.addEventListener is not a function
    at Object.eval (webpack://__ember_auto_import__/../ember-shepherd/node_modules/body-scroll-lock/lib/bodyScrollLock.min.js?:4:405)
    at eval (webpack://__ember_auto_import__/../ember-shepherd/node_modules/body-scroll-lock/lib/bodyScrollLock.min.js?:3:37)
    at eval (webpack://__ember_auto_import__/../ember-shepherd/node_modules/body-scroll-lock/lib/bodyScrollLock.min.js?:4:118)
    at Object.../ember-shepherd/node_modules/body-scroll-lock/lib/bodyScrollLock.min.js (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor.js:95681:1)
    at __webpack_require__ (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor.js:95515:30)
    at Module.eval [as callback] (webpack://__ember_auto_import__//private/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/cache-198-bundler/staging/app.js?:15:51)
    at Module.exports (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:106:1)
    at Module._reify (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:143:1)
    at Module.reify (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:130:1)
    at Module.exports (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:104:1)
    at Module._reify (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:143:1)
    at Module.reify (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:130:1)
    at Module.exports (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:104:1)
    at requireModule (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/vendor/loader/loader.js:27:1)
    at Class._extractDefaultExport (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/addon-tree-output/ember-resolver/resolvers/classic/index.js:422:1)
    at Class.resolveOther (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/addon-tree-output/ember-resolver/resolvers/classic/index.js:103:1)
    at Class.resolve (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/addon-tree-output/ember-resolver/resolvers/classic/index.js:163:1)
    at _resolve (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:1210:1)
    at Registry.resolve (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:749:1)
    at Registry.resolve (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:754:1)
    at _has (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:1227:1)
    at Registry.has (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:848:1)
    at Registry.proto.validateInjections (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:1155:1)
    at FactoryManager.create (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:524:1)
    at instantiateFactory (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:358:1)
    at _lookup (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:290:1)
    at Container.lookup (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/container.js:134:1)
    at Class.lookup (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/runtime/lib/mixins/container_proxy.js:78:1)
    at Class.<anonymous> (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/route.js:1931:1)
    at ComputedProperty.get (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/metal.js:2966:1)
    at _get (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/metal.js:1591:1)
    at Class._getQPMeta (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/router.js:1027:1)
    at Class._queryParamsFor (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/router.js:1060:1)
    at forEachQueryParam (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/router.js:1673:1)
    at Class._deserializeQueryParams (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/router.js:889:1)
    at getFullQueryParams (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/route.js:1650:1)
    at getQueryParamsFor (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/route.js:1663:1)
    at Class.paramsFor (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/route.js:206:1)
    at Class._paramsFor (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/route.js:2290:1)
    at Class.deserialize (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/@ember/-internals/routing/lib/system/route.js:1059:1)
    at UnresolvedRouteInfoByParam.getModel (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/router_js.js:965:1)
    at /var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/router_js.js:731:1
    at tryCatcher (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/rsvp.js:334:1)
    at invokeCallback (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/rsvp.js:507:1)
    at publish (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/rsvp.js:493:1)
    at /var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/ember-testing/lib/ext/rsvp.js:17:1
    at invokeWithOnError (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/backburner.js:340:1)
    at Queue.flush (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/backburner.js:223:1)
    at DeferredActionQueues.flush (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/backburner.js:427:1)
    at Backburner._end (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/backburner.js:1003:1)
    at Backburner._boundAutorunEnd (/var/folders/yt/kw3pd9l52sd9bbpjrlpm47rm0000gn/T/broccoli-94240Whgv3PCEb1E/out-201-broccoli_merge_trees/assets/backburner.js:643:1)
    at processTicksAndRejections (internal/process/task_queues.js:86:5)

@RobbieTheWagner
Copy link
Owner

Looks like it is from body-scroll-lock. That's not from Shepherd, it is here in ember-shepherd https://github.com/shipshapecode/ember-shepherd/blob/master/package.json#L59

We should probably move the disableScroll functionality into Shepherd as well, then this would be fixed.

@st-h
Copy link
Contributor Author

st-h commented Apr 29, 2019

I think moving disableScroll to shepherd.js is a nice idea. That way we can also lazy load it without having to do anything :D

@st-h st-h mentioned this issue Jun 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

load shepherd.js lazily using ember-auto-import st-h/ember-shepherd
2 participants
@RobbieTheWagner @st-h