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

support html formatting #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hatemhosny
Copy link

@hatemhosny hatemhosny commented Dec 8, 2020

This pull request adds support for html formatting (as discussed in #2)

the html parser is used when the editor language is html
(editor options, url query string, etc.)
e.g. monacode.live?language=html

The html parser is added to the bundle which slightly increases its size

The other alternative approach would be dynamically importing the parser (babel/html) based on the editor language, so only the required parser file is downloaded. This will decrease bundle size but will force the default export to be an async function (which adds a layer of complexity for end users). In addition, it outputs a warning on build that the dynamic import is not bundled.

let me know what you think.

closes #2


This change is Reviewable

@lukejacksonn
Copy link
Owner

This looks good but its hard to see exactly what changed with all the prettier changes in there. My bad for not adding a prettierrc. Could you either add one of those with single quotes for for now or save without formatting?

@hatemhosny
Copy link
Author

sorry, I did not notice that :)
I added a new commit without formatting

@lukejacksonn
Copy link
Owner

Nice, thank you 🙇 I will pull this down later and test it out!

@lukejacksonn
Copy link
Owner

Hey man, sorry for the delay getting back to you. I have had a lot on my plate recently.

So I pulled this down and tested it out.. it works great! You make a very good point about the dynamic importing of the appropriate prettier parser though.

Given that prettifying isn't critical to loading the module, perhaps we could get away without making the whole module async and rather have it so that you can't prettify until the correct parser has loaded. We could even only trigger the import after the first "save" (cmd+s) is initiated.

What are your thoughts about this kind of approach?

@hatemhosny
Copy link
Author

hi @lukejacksonn, sorry about the delay.

I agree, it is better avoiding turning the module to async.

I like more the approach of dynamically loading the parser once the editor has loaded (since we already know the language), and then we make sure that the parser has loaded before formatting.

I think waiting till the first save to start loading the parser may not be ideal. The babel parser is not a small file, and probably the user may notice some lag with this approach.

If you agree on that, I will add another commit with the modification.

@lukejacksonn
Copy link
Owner

No worries and thanks for your patience here, I have been busy with another project recently. I agree with your summary. Dynamic loading of the formatter once we know the language feels like the optimal way forward 👍

@hatemhosny
Copy link
Author

I have been experimenting with that in a side project inspired by monacode and have implemented what we have agreed upon here.
Once that is ready I will share it here and if you agree on the approach I will update the PR with the required changes.
Thanks for the great project 👍

@hatemhosny
Copy link
Author

So, I have released my project: LocalPen, and I gave credit to monacode. Thank you again.

This is how I implemented the "lazy-loaded" formatter: https://github.com/hatemhosny/localpen/blob/develop/src/localpen/formatter.ts

It can be used like that:

const formatter = createFormatter({baseUrl: '/path/'});

// whenever you are ready to load formatter (e.g. after loading the editor)
// loads prettier standalone, and optionally the parsers for languages you pass
await formatter.load([]);

// load specific language parser
await formatter.loadParser('javascript')

// format
// editor is the monaco editor instance
const formattedCode = await formatter.format(editor, 'javascript')



// languages are defined like that
const languages = [
  {
    name: 'html',
    parser: {
      name: 'html',
      pluginUrls: ['vendor/prettier/parser-html.mjs'],
    },
  },
    {
    name: 'javascript',
    parser: {
      name: 'babel',
      pluginUrls: ['vendor/prettier/parser-babel.mjs', 'vendor/prettier/parser-html.mjs'],
    },
  },
];

each language can accept an array of prettier plugins. This allows for example formatting html code inside javascript strings like here

please let me know what you think

@lukejacksonn
Copy link
Owner

Wow, that is an awesome project! Exactly the kind of thing I'd hope somebody would make with monacode.

I like the approach you have gone for too. Will probably adopt something similar next time I pick this project up, unless you beat me to it. Were you still wanting to see this functionality merged in upstream?

@hatemhosny
Copy link
Author

Thank you :)
sure, I'll work on the PR to reflect the suggested changes.

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

Successfully merging this pull request may close these issues.

html formatting
2 participants