Bundle Prism and use react-live's Editor instead #389
Conversation
The latter one changed due to the react-live upgrade
src/utils/prismImport.js
Outdated
| @@ -0,0 +1,48 @@ | |||
| /* eslint-disable filenames/match-regex */ | |||
There was a problem hiding this comment.
Why did we disable filenames for this file?
There was a problem hiding this comment.
The rule didn’t like something about the prism core import :(
There was a problem hiding this comment.
It's complaining about src/utils/prismImport.js name of this file, not the imports within the code I believe.
There was a problem hiding this comment.
Actually, all the prismjs imported file names abide by the lint rule we have for this repo 😛
There was a problem hiding this comment.
Oh, ok. Sorry, I’ll make a change
| @@ -0,0 +1,48 @@ | |||
| /* eslint-disable filenames/match-regex */ | |||
|
|
|||
| import 'prismjs/components/prism-core'; | |||
There was a problem hiding this comment.
How much weight does this add to the bundle seeing that we're including "everything"?
There was a problem hiding this comment.
All in all if we take the other React-live PR into account it’s a net minus altogether:
before the react-live and codepane refactors: 2.4M (min, non-gzip, without prism) and after: 1.6M (min, non-gzip, with prism)
There was a problem hiding this comment.
How much is just the prism part if you "external" it?
There was a problem hiding this comment.
@ryan-roemer so, it's tough to exclude prism without excluding buble as both are also imported by react-live with the respective prism subpaths as well, but if I exclude: prism, react-live, and (thus also) buble, then I land at ~1.33MB
There was a problem hiding this comment.
So that's just a ~0.3MB min difference? If so, I'm totally cool with that...
docs/tag-api.md
Outdated
| |source| PropTypes.string| String of code to be shown | | ||
|
|
||
| You can change your syntax highlighting theme by swapping the prism.js CSS file in `index.html`. Prism.js supports `Markup`, `CSS`, `C-like`and `Javascript` languages out of the box. To add more [supported languages](http://prismjs.com/#languages-list), you have to load the respective language's prism.js plugin in your index.html. | ||
| Prism.js supports `Markup`, `CSS`, `C-like`and `Javascript` languages out of the box. To add more [supported languages](http://prismjs.com/#languages-list), you have to import the respective language's prism.js component/plugin from `prismjs/components/prism-javascript`. |
There was a problem hiding this comment.
It looks like you're importing a number of prism components, so you may want to modify this to match.
There was a problem hiding this comment.
good point, I'll clarify this a little
|
@carlospaelinck where did you encounter these errors? I can take a look and see whether it outputs any warnings/errors w/ this PR Edit: Ok, I see what you mean. That's not fixed but I'll make the change |
|
@philpl Yeah that sounds fine! |
| <link href="https://fonts.googleapis.com/css?family=Open+Sans+Condensed:300,700" rel="stylesheet" type="text/css"> | ||
| <link href="https://unpkg.com/prismjs@1/themes/prism-tomorrow.css" rel="stylesheet" type="text/css"> | ||
| <link href="https://unpkg.com/normalize.css@7/normalize.css" rel="stylesheet" type="text/css"> | ||
| <link href="https://unpkg.com/spectacle/lib/themes/default/index.css" rel="stylesheet" type="text/css"> |
There was a problem hiding this comment.
Can we update the spectacle unpkg link to include a major semver range? Here we'll proactively get ahead of things with our anticipated major bump to 4:
<link href="https://unpkg.com/spectacle@^4/lib/themes/default/index.css" rel="stylesheet" type="text/css">
...
<script src="https://unpkg.com/spectacle@^4/dist/spectacle.js"></script>
<script src="https://unpkg.com/spectacle@^4/lib/one-page.js"></script>
There was a problem hiding this comment.
Done :) c190c00
The links will obviously not work until we release v4
|
@carlospaelinck I've got a follow up PR for the props issue here: #391 (includes a fix for the key propagation) |
5 participants
Fix #388
1.8.0-1i.e.spectacletestrelease which includes alanguageprop and externalised prism buildsrc/utils/prism-importfile (This includes all syntaxes that we'll support out of the box)Breaking Changes:
codePanetheme's properties have been changed towrapperandeditor. They could staypreandcode, but this wouldn't reflect the actual elements accurately.Future:
It'd be great to add some instructions on how to add more prism components. The added text to the Readme is minimal but doesn't really say much.
It'd be great to make the playground and codepane's theme props only recognise
lightanddarkso that it can be set to another value so the user can write/use their own themes.Improvements Summary:
So while the bundle will grow, since more prism components are now included (which doesn't matter much since removing codemirror and babel-standalone for buble should've helper it a little), this gets rid of extra setup steps for the user. Previously a Prism CSS and CDN bundle would have to be added manually.
Also, this should make the syntax theme between the playground and the codepane 100% consistent.