-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Inject requirejs in notebook and start using it. #3364
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.
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
Conversation
I was thinking about when we want to make this transition. We have a On Sun, May 26, 2013 at 12:44 PM, Matthias Bussonnier
|
I would agree not to do it before 1.0, whic is too much work. But having require.js will help a lot for extensions. By trying to put it into the main code base (just for testing purpose) I saw that it was breaking marked. So this mean that people who would like to inject requirejs for plugin could actually break the notebook which is annoying. So I'm not proposing to do it 100%, but at least have it injected plus the minimal modification to be sure things are not broken with it. |
I'm not sure I understand what the decision is here - it sounds like we don't want to merge this for 1.0, but there is talk of something partial, and I don't see what that partial step is. |
No, by this I mean, completely move all modules to require.js , which is obviously to much work. What I propose is to add require into the component submodules, add in to the loaded script, and fixed what need to be fixed for marked to work. Basically this PR plus a few stuff to avoid attaching marked to |
so this PR attaches marked to the window, but then you want to do another PR to undo that? |
No, I want to improve it by passing marked around, or required it in |
Still in progress, would like to clean/squash a little before merging if people still agree.
TODO:
|
// as injecting require.js make marked not to put itself in the globals, | ||
// which make both this file fail at setting marked configuration, and textcell.js | ||
// which search marked into global. | ||
require(['static/components/marked/lib/marked.js'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this must be allowed to be static_url
as in the HTML templates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done by setting require search root as {{static_url()}} in templates.
Nice, thanks. If you have it all squared away with submodules hash, etc., I'm 👍 for merge now. |
I'd like to play with it a bit. especially re-read the doc of require and its configuration. |
works for me. |
We'll make sure this goes in before attempting to merge #3395 (or its successor if we end up redoing it from scratch). |
If it does not make it (I'll try to play with it today to be sure) it would not be too painfull to rebase. |
Works nicely, also with min's bootstrap. Works also great with custom.js where you can use stuff like:
Merging ! Thanks. |
Inject requirejs in notebook and start using it. Mainly because the behavior of Marked change when require is injected. So only apply the modification needed for marked to behave.
Inject requirejs in notebook and start using it. Mainly because the behavior of Marked change when require is injected. So only apply the modification needed for marked to behave.
Not sure if we want to inject requirejs in one step everywhere or step-by-step in multiple PR each of which would refactor only a few files.