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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugins and dependency injection #5438

Open
bd82 opened this issue Nov 10, 2018 · 4 comments
Open

Plugins and dependency injection #5438

bd82 opened this issue Nov 10, 2018 · 4 comments
Labels
area:api Issues with Prettier's Application Programming Interface area:plugin api status:needs discussion Issues needing discussion and a decision to be made before action can be taken

Comments

@bd82
Copy link
Contributor

bd82 commented Nov 10, 2018

Hello.

I am trying to give a boost to the prettier-java plugin which was moved to the Jhipster
organization.

I am trying to understand the relationship between a plugin and prettier itself.
I see that most of the plugins listed in: https://github.com/prettier/prettier/blob/master/docs/plugins.md
specify a direct dependency to prettier in their package.json.

I also see that the plugin development guide:

const { concat, join, line, ifBreak, group } = require("prettier").doc.builders;

Does it make sense that a plugin depends on the core platform directly?
Won't that cause multiple versions of prettier to exist at the same time?
Should not the core platform load the plugin and provide the APIs in some kind of DI injection style?
e.g:

// This function get invoked by prettier when it loads the discovered plugins
function initBuilders(prettier_core_builders) {
   const { concat, join, line, ifBreak, group } = prettier_core_builders;
}

module.exports = { initBuilders };

Thanks.
Shahar.

@j-f1
Copy link
Member

j-f1 commented Nov 10, 2018

As long as you use ^-style ranges, you shouldn鈥檛 have issues with duplicated Prettier versions.

@j-f1 j-f1 added status:needs discussion Issues needing discussion and a decision to be made before action can be taken area:api Issues with Prettier's Application Programming Interface labels Nov 10, 2018
@ikatyang
Copy link
Member

Yeah, I thought about it before (recursive dependency). But since nobody raised this kind of issues, I didn't investigate it further. We should change it to not directly depend on prettier, though it'll make the development inconvenient (we cannot do top-level require to get doc-related method, we need to pass it to every function that needs this functionality).

@j-f1
Copy link
Member

j-f1 commented Nov 11, 2018

How about creating a @prettier/doc-builders package that both core and plugins depend on?

@bd82
Copy link
Contributor Author

bd82 commented Nov 11, 2018

As long as you use ^-style ranges, you shouldn鈥檛 have issues with duplicated Prettier versions.

That would be much better and I will apply it, but it only solves the problem for minor versions not major versions.

How about creating a @prettier/doc-builders package that both core and plugins depend on?

This seems like a better way, but if the internal structure of the Doc changes it would be a breaking change. For example: if pluginX uses doc-builders@1.2.0 and prettier uses doc-builders@2.1.0 and those two versions produce a different Doc object than we will have a problem.

On the other hand if the interface for building a Doc object:

  • Is Well defined.
  • Won't change in prettier major version changes.
  • will be Injected to plugins.

It would mean that plugins could even be re-used across major versions of prettier.
So if/when prettier 2.0 is released due to some breaking change in the consumer API (unrelated to plugins). It would not require all plugins to release new versions too.

bd82 added a commit to jhipster/prettier-java that referenced this issue Nov 11, 2018
to reduce cases of duplicate prettier versions.

See: prettier/prettier#5438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Issues with Prettier's Application Programming Interface area:plugin api status:needs discussion Issues needing discussion and a decision to be made before action can be taken
Projects
None yet
Development

No branches or pull requests

4 participants