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.

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

move html serializer out of core #55

Closed
ianstormtaylor opened this issue Jul 7, 2016 · 4 comments
Closed

move html serializer out of core #55

ianstormtaylor opened this issue Jul 7, 2016 · 4 comments

Comments

@ianstormtaylor
Copy link
Owner

Adds too much opinion, and also bloat with cheerio dependency.

@thesunny
Copy link
Collaborator

Hi @ianstormtaylor

I'm thinking of tackling this one. Feels like it provides some good ROI in terms of reducing file size. I also noticed that Cheerio has a dependency on lodash which I think, previously, somebody took the liberty of removing from slate itself.

The question I have is what should I do with the html serializer? Should I just open up my own repo and have you fork from it? I don't want to be the core maintainer for it so what's the best way to get it into your hands?

Also, I'm wondering if you might prefer another option where the HTML serializer is available in a subpath.

For example:

import {Editor} from 'slate'

// but to get the HTML serializer
import HTML from 'slate/html-serializer'

Under this scenario, the entire library size won't change but the amount of code being loaded into the browser would be less. Only if you import from slate/html-serializer would cheerio and its dependencies be loaded. The pro would be that it would be easier to keep the HTML serializer in sync with Slate and you wouldn't have versioning issues (which version of the serializer for which version of Slate if you are using an older version).

@thesunny
Copy link
Collaborator

Along the same line, if we do decide to move the html serializer to a subpath, we could easily do the same thing with the plain serializer and the raw serializer. This means they wouldn't need to get loaded unless they were actually needed.

@ianstormtaylor
Copy link
Owner Author

Hey @thesunny, interesting idea about the file paths. I hadn't thought about that. I think it might be something we could consider later. My only hesitation is that I think Raw and Plain are pretty small, and used in some of the core logic anyways, so they wouldn't necessarily be big saves.

But I definitely still want to allow us to slim down the Html serializer since it's big. I haven't gotten to this yet, sorry for the delay, but I think it's probably easiest if I do it myself for future maintaining reasons. Thank you for offering to help though!

@thesunny
Copy link
Collaborator

Makes sense and sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants