Support async compilers about unified HOT 31 CLOSED

This is the same discussion as eslint-plugin-prettier vs prettier-eslint or stylelint-prettier vs prettier-stylelint. Some people prefer one solution, others prefer another, and that’s ok.

Anyway, I’ll close this issue, as @JounQin’s solution to use @prettier/sync will solve it. I will also split remark-prettier into two plugins as described in #196 (comment) once Prettier 3 is officially released.

from unified.

I don’t understand why they are done as a plugin.

This is how most tools work:

|     TS     |     babel   |    eslint    |    prettier   |
|            |             |              |               |
|    tree    |     tree    |     tree     |     tree      |
|   /    \   |    /    \   |    /    \    |    /    \     |
|  /      \  |   /      \  |   /      \   |   /      \    |
|string    string        string        string       string|

They can be glued together with Gulp or Grunt or bash scripts or whatever. They work on strings.

This is how unified works:

|                       unified                           |
|   parse    |  transform  |     check    |    format     |
|            |             |              |               |
|    tree -------- tree -------- tree --------- tree      |
|   /                                               \     |
|  /                                                 \    |
|string                                             string|

It works on trees.
Some new things try to do the same, such as Rome.
I believe this is the benefit that unified has over Gulp/Grunt/bash scripts.
The reason it exists.

I don’t understand why this has to be solved in unified instead of:

import unified from 'unified'
import {prettier} from 'vfile-prettier';
import {reporter} from 'vfile-reporter'
import {read} from 'to-vfile'

const file = await unified()
  // ...
  .process(await read('readme.md'))

await prettier(file)

// Formatted content
console.log(String(file));

// Prettier formatting violations
console.error(reporter(file));

from unified.

I understand what it does!

I don’t think it’s a good idea in unified itself.

There are differences between eslint and prettier and gulp and grunt etc and unified (as shown in the diagram above), which is why I don’t think it makes sense as a unified plugin

from unified.

Personally, I love the idea remark-prettier.

Because even with https://github.com/remarkjs/remark-preset-prettier, remark-stringify's default or few custom formatting can still change the input.

For example, there is no option to preserve two spaces:

Line.  <!-- two spaces here -->
Next line

It will always to formated as:

Line.\
Next line

This is very unexpected for prettier users.

from unified.

Vite should respect the worker export condition when using web workers, but it doesn’t. See vitejs/vite#7439

from unified.

• deasync is indeed not nice.
• I don’t see while serializing a tree, for unified, should ever be async
• Should prettier really be run as a unified compiler? It doesn’t use the tree, which is the whole thing that unified provides. Why not run prettier afterwards?

from unified.

remark-prettier could use deasync, but IMO that’s not a nice solution.

You should at least use https://github.com/un-ts/synckit

eslint-mdx and many other eslint/prettier plugins are using it.

• https://github.com/ota-meshi/eslint-plugin-json-schema-validator
• https://github.com/ota-meshi/eslint-plugin-node-dependencies
• https://github.com/withastro/prettier-plugin-astro
• https://github.com/ersin84/prettier-plugin-wromo

from unified.

remark-prettier offers the same thing that eslint-plugin-prettier offers: A single run (through remark CLI or language server) formats the code as desired, including Prettier formatting as well as other remark transforms.

Now that I’m thinking about it, this could also be resolved by unified-args and unified-language-server better supporting autofixes based on expected. This way no compiler is necessary.

from unified.

prettier@v3 will have a sync mode via worker_threads also: https://github.com/prettier/sync

But that is lack of features, yarn PnP support, for example: prettier/prettier-synchronized#2

from unified.

@JounQin thanks for the hint! I’d still rather avoid forcing async code into synchronous, but synckit does look nicer than deasync because it doesn’t appear to use Node.js C++ bindings.

from unified.

eslint-plugin-prettier needs to adopt async version of prettier@v3 into sync due to ESLint limitation at the same time.

from unified.

@remcohaszing You have two plugins in one.

The first plugin assumes markdown is input (and that no plugin adds content to it).
It passes that through prettier, fine.
This could be useful as a plugin indeed.
But it doesn’t depend on the AST.

So this could in my opinion better be a separate integration that accepts one (or more) vfiles, and accepts all the languages that prettier supports besides markdown, and generates lint messages for them.

The second plugin duplicates what remark-stringify does, and then does the same as what unified does with prettier: parse a file to a prettier tree, do things on the prettier tree, serialize the tree prettier.
There is no reason to inline remark-stringify. And the same solution as the other plugin exists: a separate integration that accepts one (or more) vfiles, accepts all the languages that prettier supports besides markdown, and reformats them

from unified.

I agree this is basically two plugins in one and should be separated into two. And both of them don’t need to be specific to mdast.

The first plugin could compile the AST using the registered unified compiler and report diffs from the current VFile contents without depending on Prettier.

The second plugin could override the compiler, compile the AST using the old compiler, then reformat the code using Prettier. This way it would be language independant.

The second plugin would still require an async compiler. @JounQin’s suggestions really help though, so maybe it’s not needed.

from unified.

@wooorm

remark-prettier is something just like eslint-plugin-prettier, it only requires a .remarkrc with

{
  "plugins": ["prettier"]
}

No code, just config.

from unified.

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

from unified.

Hi team! Could you describe why this has been marked as wontfix?

Thanks,
— bb

from unified.

I still want to discuss remark-stringify's formatting behavior. It is really unexpected to change unrelated codes when using remark-lint. 😂

Or maybe a new discussion/issue should be raised.

from unified.

This is the same discussion as eslint-plugin-prettier vs prettier-eslint or stylelint-prettier vs prettier-stylelint. Some people prefer one solution, others prefer another, and that’s ok.

Somewhat. The main thing for me is: should unified be Gulp? Gulp lets you stitch every tool together through wrapper plugins.
I don’t believe so. Wrapper plugins are hard to maintain. They are always upgraded to new major versions later than the actual tool. And they often go unmaintained, with lots of bugs.

Regardless, Gulp like npm are task runners. They allow different tools to work together. That‘s a layer above unified. unified doesn’t have to focus on that.

To reiterate my previous diagrams, this is unified:

|                       unified                           |
|   parse    |  transform  |     check    |    format     |
|            |             |              |               |
|    tree -------- tree -------- tree --------- tree      |
|   /                                               \     |
|  /                                                 \    |
|string                                             string|

And this is gulp/npm scripts:

|     TS     |     babel   |    eslint    |    prettier   |
|            |             |              |               |
|    tree    |     tree    |     tree     |     tree      |
|   /    \   |    /    \   |    /    \    |    /    \     |
|  /      \  |   /      \  |   /      \   |   /      \    |
|string    string        string        string       string|

(and of course gulp/npm also support tools that don’t operate on ASTs and do other fancy things).

Lastly, unified is two things. It‘s this tiny package, and the whole ecosystem.
There are different things that other packages can do, e.g., unified-engine depends on Node and is more of a “batteries included” thing.
As this issue is here, me not thinking this issue fits here is one thing. Similar things make more sense in higher level projects.

from unified.

• deasync is indeed not nice.

• I don’t see while serializing a tree, for unified, should ever be async

• Should prettier really be run as a unified compiler? It doesn’t use the tree, which is the whole thing that unified provides. Why not run prettier afterwards?

We live in a world we're many libs have to be async so as to not hang UI. MermaidJS is a good example. It's render function is async. You have to expect that some capabilities need to be async and if you ever introduce async anywhere in the code path and need to execute synchronously you are out of luck. I hope this helps.

It would be wise to allow for async plugins and would allow more innovation with unified.

from unified.

I don’t understand why they are done as a plugin.

This is how most tools work:

|     TS     |     babel   |    eslint    |    prettier   |

|            |             |              |               |

|    tree    |     tree    |     tree     |     tree      |

|   /    \   |    /    \   |    /    \    |    /    \     |

|  /      \  |   /      \  |   /      \   |   /      \    |

|string    string        string        string       string|

They can be glued together with Gulp or Grunt or bash scripts or whatever. They work on strings.

This is how unified works:

|                       unified                           |

|   parse    |  transform  |     check    |    format     |

|            |             |              |               |

|    tree -------- tree -------- tree --------- tree      |

|   /                                               \     |

|  /                                                 \    |

|string                                             string|

It works on trees.

Some new things try to do the same, such as Rome.

I believe this is the benefit that unified has over Gulp/Grunt/bash scripts.

The reason it exists.

I don’t understand why this has to be solved in unified instead of:

import unified from 'unified'

import {prettier} from 'vfile-prettier';

import {reporter} from 'vfile-reporter'

import {read} from 'to-vfile'



const file = await unified()

  // ...

  .process(await read('readme.md'))



await prettier(file)



// Formatted content

console.log(String(file));



// Prettier formatting violations

console.error(reporter(file));

You make a great and valid point with this example. One reason to use unified is to take full advantage of many plugins and not have one off solution. If everything is a plugin that increases reuse and allows flexibility in the ordering of plugin execution to perform transformation. Unified per its namesake should unify how things are done. I want to use mermaid, prism, and pettier using a unified and intuitive plugin api. I think that's reasonable. I am developing a web app and today I don't have the ability to use other tools to transform markdown documents and render mermaid, and highlight code, etc all in one shot.

from unified.

It would be wise to allow for async plugins and would allow more innovation with unified.

Plugins can be async: https://github.com/unifiedjs/unified#function-transformertree-file-next.
You can use mermaid, prism in unified fine already.

This issue was about compilers, whose responsibility is ast -> string. I think that can be done sync. Can you show an example where that is not the case?

from unified.

It would be wise to allow for async plugins and would allow more innovation with unified.

Plugins can be async: https://github.com/unifiedjs/unified#function-transformertree-file-next. You can use mermaid, prism in unified fine already.

This issue was about compilers, whose responsibility is ast -> string. I think that can be done sync. Can you show an example where that is not the case?

It blocks the main thread in web apps tho?

from unified.

Then don’t do it on the main thread in web apps.

from unified.

I don't think it works out of box, tested yesterday, it complained about lacking document object/dom or something, in the web worker. I am completely new to web workers tho.

from unified.

This seems to cause an error because document is not available in the web worker:
https://www.npmjs.com/package/decode-named-character-reference

from unified.

@vlrevolution that will depend on your framework and your build tool.
Based on your comments on ssssota/svelte-exmarkdown#209 and bytedance/bytemd#35 you are using svelte with an unknown build tool.
The answer would be svelte specific, you may want to use sveltejs/svelte#10738 (comment) as inspiration.

from unified.

This seems to cause an error because document is not available in the web worker

Configure your build tool, a web worker version is provided, but you need to actually use it https://github.com/wooorm/decode-named-character-reference/blob/4347630741f82d7d4c7fb38763ba158951b2a1be/package.json#L36

from unified.

This seems to cause an error because document is not available in the web worker

Configure your build tool, a web worker version is provided, but you need to actually use it https://github.com/wooorm/decode-named-character-reference/blob/4347630741f82d7d4c7fb38763ba158951b2a1be/package.json#L36

I use vite for build. Thank you for the pointers tho, they help a lot!

What exactly does this mean? It seems kind of abstract to me, how to make vite use this worker version instead?

from unified.

Any way to override it?

from unified.

No, but there are other work arounds, see vitejs/vite#7439 and try the suggestions there

from unified.

No, but there are other work arounds, see vitejs/vite#7439 and try the suggestions there

Many thanks, it is working!

For reference to others I put this in my vite.config.ts:

	resolve: {
		conditions: ['worker']
	},

from unified.