Prettier Plugin API #3536

azz · 2017-12-19T22:41:58Z

Very early progress - ~~I've only moved files so far~~. Want to keep the build green and get as much feedback as possible on this one.

Closes #3511

Follow ups (2.0 items):

Split out the support info and options to be language-specific
Make it externally configurable via CLI
Make it externally configurable via options/configuration file
Switch to monorepo to enforce correct imports (?)
Use prettier-python as a proof-of-concept

lydell · 2017-12-20T11:27:08Z

src/builder/util.js

+    node &&
+    node.comments &&
+    node.comments.length > 0 &&
+    node.comments.some(comment => comment.value.trim() === "prettier-ignore")


FYI [].some(() => true) === false, so node.comments.length > 0 is not needed other than for explicitness.

True. I just moved this code though 😉

azz · 2017-12-20T11:58:57Z

Managed to extract all the generic printing logic from the old printer.js file into main/ast-to-doc.js. Only one failing test! I suspect this is due to the doc being wrapped in less concat()s.

Test failure

$ PRETTIER_DEBUG=1 yarn test template_literals
yarn test v1.0.2
$ jest "template_literals"
 FAIL  tests/template_literals/jsfmt.spec.js
  ● Test suite failed to run

    Couldn't insert all the expressions

      at transformCssDoc (src/main/multiparser.js:386:11)
      at fromBabylonFlowOrTypeScript (src/main/multiparser.js:106:16)
      at Object.printSubtree (src/main/multiparser.js:20:14)
      at genericPrint (src/main/ast-to-doc.js:83:31)
      at comments.printComments.p (src/main/ast-to-doc.js:38:14)
      at Object.printComments (src/main/comments.js:1102:19)
      at printGenerically (src/main/ast-to-doc.js:36:22)
      at FastPath.call (src/builder/fast-path.js:70:18)
      at printPathNoParens (src/language-js/printer-estree.js:1698:29)
      at Object.genericPrint [as print] (src/language-js/printer-estree.js:55:30)
      at genericPrint (src/main/ast-to-doc.js:96:18)
      at comments.printComments.p (src/main/ast-to-doc.js:38:14)
      at Object.printComments (src/main/comments.js:1102:19)
      at printGenerically (src/main/ast-to-doc.js:36:22)
      at FastPath.map (src/builder/fast-path.js:124:19)
      at printJSXElement (src/language-js/printer-estree.js:4310:19)
      at comments.printComments (src/language-js/printer-estree.js:1717:15)
      at Object.printComments (src/main/comments.js:1102:19)
      at printPathNoParens (src/language-js/printer-estree.js:1715:29)
      at Object.genericPrint [as print] (src/language-js/printer-estree.js:55:30)
      at genericPrint (src/main/ast-to-doc.js:96:18)
      at printGenerically (src/main/ast-to-doc.js:34:13)
      at FastPath.call (src/builder/fast-path.js:70:18)
      at printPathNoParens (src/language-js/printer-estree.js:225:14)
      at Object.genericPrint [as print] (src/language-js/printer-estree.js:55:30)
      at genericPrint (src/main/ast-to-doc.js:96:18)
      at comments.printComments.p (src/main/ast-to-doc.js:38:14)
      at Object.printComments (src/main/comments.js:1102:19)
      at printGenerically (src/main/ast-to-doc.js:36:22)
      at path.map (src/language-js/printer-estree.js:2846:25)
      at FastPath.map (src/builder/fast-path.js:124:19)
      at printStatementSequence (src/language-js/printer-estree.js:2830:8)
      at parts.push.path.call.bodyPath (src/language-js/printer-estree.js:201:18)
      at FastPath.call (src/builder/fast-path.js:70:18)
      at printPathNoParens (src/language-js/printer-estree.js:200:14)
      at Object.genericPrint [as print] (src/language-js/printer-estree.js:55:30)
      at genericPrint (src/main/ast-to-doc.js:96:18)
      at comments.printComments.p (src/main/ast-to-doc.js:38:14)
      at Object.printComments (src/main/comments.js:1102:19)
      at printGenerically (src/main/ast-to-doc.js:36:22)
      at printAstToDoc (src/main/ast-to-doc.js:51:13)
      at formatWithCursor (index.js:112:15)
      at format (index.js:137:10)
      at Object.format (index.js:376:12)
      at prettyprint (tests_config/run_spec.js:123:19)
      at fs.readdirSync.forEach.filename (tests_config/run_spec.js:47:22)
          at Array.forEach (<anonymous>)
      at run_spec (tests_config/run_spec.js:21:27)
      at Object.<anonymous> (tests/template_literals/jsfmt.spec.js:1:90)
          at Generator.next (<anonymous>)
          at new Promise (<anonymous>)
          at Generator.next (<anonymous>)
          at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:188:7)

azz · 2017-12-20T12:12:52Z

Any idea what happened to this job? https://travis-ci.org/prettier/prettier/jobs/319135394

ikatyang · 2017-12-20T14:14:18Z

src/main/parser.js

  },
  get markdown() {
-    return eval("require")("./parser-markdown");
+    return eval("require")("../language-markdown/parser-markdown");


In bundled version: Cannot find module '../language-markdown/parser-markdown' from 'index.js'

Yeah I'll be moving the output too eventually. Will have to upgrade the playground too.

azz · 2017-12-22T09:13:11Z

As of 35c57c6 Prettier uses the plugin API internally! 🎉

Still have to get the build green (1 failing multiparser test remains), but should be good to review now.

j-f1 · 2017-12-22T11:01:17Z

Why do the language files use eval("require")?

azz · 2017-12-22T11:35:22Z

It's a gross hack to prevent Rollup from bundling in the parsers. I'm just moving it from the parsers.js file. Would probably be better to use Rollup's external as a function, but we can do that in a separate PR.

ikatyang · 2017-12-22T13:24:41Z

bin/prettier.js

@@ -2,4 +2,4 @@

 "use strict";

-require("../src/cli").run(process.argv.slice(2));
+require("../src/cli/cli").run(process.argv.slice(2));


How about src/cli/index?

ikatyang · 2017-12-22T13:40:00Z

src/language-markdown/index.js

+  get parse() {
+    return eval("require")("./parser-markdown");
+  },
+  astFormat: "remark"


ikatyang · 2017-12-22T13:46:22Z

src/main/doc-to-string.js

@@ -1,7 +1,7 @@
 "use strict";



main/doc-to-string -> builder/doc-printer?

I think they should be in the same group since they are parts of the core printer, just like a standalone package.

doc-builders doc-printer doc-utils fast-path

Good idea. Renamed it to doc to make it a more cohesive package name. Moved out fast-path as it doesn't really belong.

Need to move FastPath#needsParens to a separate file in language-js I think.

ikatyang · 2017-12-23T08:14:22Z

We probably need to split out more things to be language-specific, e.g.

massageAST
- external plugin currently has no ability to use it, --debug-check will always throw errors.
hasPragma/insertPragma
- they're currently only available (valid) in JS/CSS
more?

j-f1 · 2017-12-23T11:47:02Z

hasPragma/insertPragma

they're currently only available (valid) in JS/CSS

Markdown:

<!-- @format -->

GraphQL:

# @format

azz · 2017-12-23T12:35:02Z

That's already a bug. If you use --insert-pragma with a GraphQL file it inserts a /** @format */ comment which is a syntax error. Is there an issue raised for it?

Edit: Raised #3557

azz · 2017-12-23T12:35:37Z

Getting this error on the production build... Feel like I've seen it before...

Summary of all failing tests
 FAIL  tests_integration/__tests__/config-resolution.js
  ● API resolveConfig with file arg
    TypeError: Cannot read property 'children' of undefined
      
      at requireFromString (dist/index.js:23817:8)
      at parseJsFile (dist/index.js:23963:15)
  ● API resolveConfig.sync with file arg
    TypeError: Cannot read property 'children' of undefined
      
      at requireFromString (dist/index.js:23817:8)
      at parseJsFile (dist/index.js:23963:15)
      at loadJs (dist/index.js:23970:7)
      at dist/index.js:24276:16
      at index.forEach.results.reduce.text.replace.split.forEach.isBetween.some.Object.assign.comments.forEach.comments.forEach.tiesToBreak.forEach.path.each.path.each.FastPath$1.needsParens.path.each.path.each.path.each.parts.push.path.each.grouped.length.standalones.length.n.specifiers.n.specifiers.some.path.each.fields.forEach.propsAndLoc.sort.path.each.path.each.parts.push.path.call.path.map.path.call.parts.push.path.each.path.each.parts.push.path.call.path.call.path.call.printedNodes.(anonymous function).node.comments.printedNodes.(anonymous function).node.comments.some.flatGroups.slice.some.path.map.n.children.children.forEach.node.comments.node.comments.some.node.comments.node.comments.some.path.call.apply.path.call.template.quasis.some.path.each.path.each.path.map.value.replace.value.replace.sequencePath.map.node.value.replace.replace.path.map.contents.reduce.path.map.util$12.mapDoc.parts.reduce.path.each.results.push.lines.map.textLine.forEach.util$8.mapDoc.docUtils$3.mapDoc.results.push.exports.printProps.exports.printChildren.plugin.serialize.Object.keys.forEach.loadPlugins$3.reduce.filter.getSupportInfo$1.languages.find.Object.keys.forEach.chainFuncsSync (dist/index.js:23825:42)
      at Array.reduce (native)
      at funcRunner (dist/index.js:23838:16)
      at searchDirectory (dist/index.js:24258:20)
      at load (dist/index.js:24248:9)
      at dist/index.js:33229:49
      at Array.map (native)
      at _resolveConfig (dist/index.js:33229:40)
      at Function.index.forEach.results.reduce.text.replace.split.forEach.isBetween.some.Object.assign.comments.forEach.comments.forEach.tiesToBreak.forEach.path.each.path.each.FastPath$1.needsParens.path.each.path.each.path.each.parts.push.path.each.grouped.length.standalones.length.n.specifiers.n.specifiers.some.path.each.fields.forEach.propsAndLoc.sort.path.each.path.each.parts.push.path.call.path.map.path.call.parts.push.path.each.path.each.parts.push.path.call.path.call.path.call.printedNodes.(anonymous function).node.comments.printedNodes.(anonymous function).node.comments.some.flatGroups.slice.some.path.map.n.children.children.forEach.node.comments.node.comments.some.node.comments.node.comments.some.path.call.apply.path.call.template.quasis.some.path.each.path.each.path.map.value.replace.value.replace.sequencePath.map.node.value.replace.replace.path.map.contents.reduce.path.map.util$12.mapDoc.parts.reduce.path.each.results.push.lines.map.textLine.forEach.util$8.mapDoc.docUtils$3.mapDoc.results.push.exports.printProps.exports.printChildren.plugin.serialize.Object.keys.forEach.loadPlugins$3.reduce.filter.getSupportInfo$1.languages.find.Object.keys.forEach.resolveConfig.sync (dist/index.js:33256:42)
      at Object.<anonymous> (tests_integration/__tests__/config-resolution.js:80:33)

ikatyang · 2017-12-24T07:16:19Z

src/language-markdown/embed.js

+
+  return null;
+
+  function getParserName(lang) {


It seems we can re-use the support info (langName/extName) to get the correct parser? and it'll work with all external plugins.

Good idea! Done.

ikatyang · 2017-12-24T07:17:01Z

package.json

@@ -47,6 +47,7 @@
    "postcss-values-parser": "1.3.1",
    "remark-frontmatter": "1.1.0",
    "remark-parse": "4.0.0",
+    "resolve": "^1.5.0",


1.5.0 😂

Not again... (at least the script catches it :D)

azz · 2017-12-24T12:51:23Z

Looks like there's a Rollup bug. loadPlugins$2 is not a function. First glance looks like Rollup has bundled in the files in the wrong order. Maybe a circular dependency? I tried upgrading Rollup but there was a stack of breaking changes and after I'd resolved it all I was getting errors with uglify-es so I gave up :(

Any ideas?

taion · 2017-12-24T16:24:57Z

src/common/load-plugins.js

+
+  const externalPlugins = options.plugins.map(plugin => {
+    const pluginPath = resolve.sync(plugin, { basedir: process.cwd() });
+    return eval("require")(pluginPath);


I wonder if there's any way this could support some sort of CLI-based API in addition to the JS ones, say some standard CLI commands that return information as JSON.

Doing so could allow writing Prettier plugins in languages other than JS, which would allow better integration for other language ecosystems.

Hypothetically, yes. But it would require an RPC interface to call the various functions (print, embed, parse, etc)

Not RPC per se since there's no remote ;)

Just like:

$ prettier-python parse <whatever>

Really this is all around prettier/plugin-python#2.

…3563) There's a lot of demand for vue sfc (#2097). This introduces partial support for them: all the html is printed as is, except for the script and style tags which are printed using prettier. I believe that this should cover a lot of the use cases while being simple to support and if we want we can extend to more in the future. I copy pasted the html parser used by vue (it's just a single 400 lines file) so that we don't run the chancesof conflicts. I'm also very conservative: I only print the style and script at the top level and for the lang attributes we support. I expect this to be landable as is and provide value, review welcome :)

azz · 2017-12-25T11:48:01Z

🎄 ✅ Build is green!

vjeux

So good! Thanks a lot for driving this, it has been a long time coming now that we support every language on earth to have a proper way of splitting them up :)

taion · 2017-12-28T00:53:22Z

With the current implementation, can plugins add their own CLI options?

At a glance, it looks like "no" – I think we'd need to do option parsing in two passes – first load the plugins, then load the config options for each plugin.

azz · 2017-12-28T02:33:34Z

Maybe just recommend using a config file for options? I'm happy to look at supporting CLI option contributions if it is easy to implement though.

taion · 2017-12-28T02:36:32Z

See https://github.com/prettier/prettier/pull/3584/files#r158743435 – it's not quite sufficient because if the option is unrecognized,

Prettier warns on the option
Prettier ignores the option

So even adding stuff to config files, disregarding anything like validation, is not sufficient. So unless I'm missing something ATM there's no actual way to configure plugins.

azz force-pushed the plugin-refactor branch from 454bb08 to 8ba7e94 Compare December 20, 2017 09:19

lydell reviewed Dec 20, 2017

View reviewed changes

azz mentioned this pull request Dec 20, 2017

Sort parsers and more #3526

Closed

ikatyang reviewed Dec 20, 2017

View reviewed changes

azz mentioned this pull request Dec 21, 2017

Initial Python formatting support #3349

Closed

9 tasks

j-f1 mentioned this pull request Dec 21, 2017

Can't use Prettier within a browser #3296

Closed

josephfrazier mentioned this pull request Dec 21, 2017

linter Plugin support for 2.0 #3547

Closed

ikatyang reviewed Dec 23, 2017

View reviewed changes

azz mentioned this pull request Dec 23, 2017

--insert-pragma breaks GraphQL, CSS #3557

Closed

ikatyang reviewed Dec 24, 2017

View reviewed changes

azz changed the title ~~[WIP] Prettier Plugin API~~ Prettier Plugin API Dec 24, 2017

taion reviewed Dec 24, 2017

View reviewed changes

azz force-pushed the plugin-refactor branch 2 times, most recently from 50ad50e to 5a10c03 Compare December 25, 2017 10:22

azz added 5 commits December 25, 2017 21:24

Move files around in preparation for refactor

c4ed408

Update paths in build script

a759a1c

Extract generic printing logic from the JavaScript printer

568baef

Conform printer API

ddd8674

Fixup decorator handling

f6b82ce

azz added 12 commits December 25, 2017 21:24

Export getParsers

9f105ac

Pin resolve version

9676761

Use getSupportInfo in Markdown embed

4f55e7d

Document plugin API

858023b

Update build-docs

c8b0bfb

Add CLI for plugins

e46f31f

Lint docs

3a4f21a

Fixup build.js

68c83a6

Add vue language

289a46c

Fixup multiparser for vue

5a10c03

Upgrade rollup and rollup-plugin-commonjs

d81b7e6

Fixup third-party build

8382c48

Change AST format in docs

52072e8

vjeux approved these changes Dec 26, 2017

View reviewed changes

azz merged commit 4c9d406 into prettier:master Dec 26, 2017

azz deleted the plugin-refactor branch December 26, 2017 01:23

azz mentioned this pull request Dec 26, 2017

Support handlebars #3534

Merged

ikatyang mentioned this pull request Dec 26, 2017

PR preview playground seems broken #3581

Closed

taion mentioned this pull request Dec 26, 2017

Actually support plugins field in config #3584

Merged

taion mentioned this pull request Dec 28, 2017

Prettier 2.0 (old) #3503

Closed

duailibe added this to the 1.10 milestone Dec 30, 2017

azz mentioned this pull request Jan 1, 2018

Use Yarn workspaces & Lerna? #3623

Open

This was referenced Mar 20, 2018

Plugin API not working with globally installed modules #4000

Closed

Fix plugin API in globally installed Prettier and introduce optional --plugin-search-dir #4192

Merged

lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 18, 2019

lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prettier Plugin API #3536

Prettier Plugin API #3536

azz commented Dec 19, 2017 •

edited

lydell Dec 20, 2017

azz Dec 20, 2017

azz commented Dec 20, 2017

azz commented Dec 20, 2017

ikatyang Dec 20, 2017

azz Dec 20, 2017

azz commented Dec 22, 2017

j-f1 commented Dec 22, 2017

azz commented Dec 22, 2017 •

edited

ikatyang Dec 22, 2017

ikatyang Dec 22, 2017

ikatyang Dec 22, 2017

azz Dec 23, 2017 •

edited

ikatyang commented Dec 23, 2017

j-f1 commented Dec 23, 2017

azz commented Dec 23, 2017 •

edited

azz commented Dec 23, 2017

ikatyang Dec 24, 2017

azz Dec 24, 2017

ikatyang Dec 24, 2017

azz Dec 24, 2017

azz commented Dec 24, 2017

taion Dec 24, 2017

azz Dec 25, 2017

taion Dec 25, 2017

azz commented Dec 25, 2017 •

edited

vjeux left a comment

taion commented Dec 28, 2017

azz commented Dec 28, 2017

taion commented Dec 28, 2017

Prettier Plugin API #3536

Prettier Plugin API #3536

Conversation

azz commented Dec 19, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azz commented Dec 20, 2017

azz commented Dec 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azz commented Dec 22, 2017

j-f1 commented Dec 22, 2017

azz commented Dec 22, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azz Dec 23, 2017 • edited

Choose a reason for hiding this comment

ikatyang commented Dec 23, 2017

j-f1 commented Dec 23, 2017

azz commented Dec 23, 2017 • edited

azz commented Dec 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azz commented Dec 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azz commented Dec 25, 2017 • edited

vjeux left a comment

Choose a reason for hiding this comment

taion commented Dec 28, 2017

azz commented Dec 28, 2017

taion commented Dec 28, 2017

azz commented Dec 19, 2017 •

edited

azz commented Dec 22, 2017 •

edited

azz Dec 23, 2017 •

edited

azz commented Dec 23, 2017 •

edited

azz commented Dec 25, 2017 •

edited