-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Update documentation for node & webpack usage #1429
Conversation
9fea07c
to
336f676
Compare
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.
Looks pretty good to me. See my comments below, altough I don't mind if the PR is merged in its current state.
directly from the <code>components</code> folder. Or you can use the <code class="language-javascript">loadLanguages()</code> utility, that | ||
will automatically handle any required dependencies.</p> | ||
<code>clike</code> and <code>javascript</code>. You can load more languages with the | ||
<code class="language-javascript">loadLanguages()</code> utility, which will automatically handle any required dependencies.</p> |
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.
Do we want to remove any reference to manual requiring? I mean it can still be useful, for example to load a custom language definition.
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.
My approach was to, for both bundlers & Node, give them One True Way™ to load Prism, to reduce confusion about how they should go about it. My assumption / expectation was manual requiring is what people are going to try before reading the documentation and I don't want to encourage manual requiring of language definitions so we don't repeat what happened in 1.14.0
with the markup-templating
change. Now that we've got solutions for handling internal language dependencies, we should be pushing people to use them.
index.html
Outdated
@@ -184,6 +197,8 @@ <h1>Basic usage</h1> | |||
// Returns a highlighted HTML string | |||
var html = Prism.highlight(code, Prism.languages.haml, 'haml');</code></pre> | |||
|
|||
<p><strong>Note</strong>: Do <em>not</em> use <code>loadLanguages</code> with Webpack or another bundler, as this will cause Webpack to include all languages and plugins. Use the babel plugin described above.</p> |
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.
We used <code class="language-javascript">loadLanguages()</code>
previously. We should probably harmonise this.
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.
👍 will fix.
336f676
to
1e99e96
Compare
@Golmote I made the one change you suggested here; if you think we should re-include manual requiring for node, let me know and I'll fix, but I'm going to merge for now. |
Fix #1403 and #1409.