[jest-docblock] add docblock.print() #4517

azz · 2017-09-20T12:19:16Z

Summary

Prettier is implementing a --insertPragma flag to add /** @format */ to docblock, and ideally we'd be able to augment existing docblocks rather than prepending a new one, which will break tools which only look at the first one.

Context: prettier/prettier#2865 (comment)

This PR adds a simple docblock.print() function.

docblock.print({pragmas: {flow: ''}}); 
/** @flow */

docblock.print({pragmas: {providesModule: "x/y"}, comments: "My module"});
/**
 * My module
 *
 * @providesModule x/y
 */

In practice:

const {pragmas} = docblock.parse("/** @flow */");
const updated = docblock.print({pragmas: {...pragmas, format: ''}});
/**
 * @flow
 * @format
 */

Test plan

Unit tests are covering most cases are included.

cpojer · 2017-09-20T12:23:42Z

I like it.

codecov-io · 2017-09-20T12:52:08Z

Codecov Report

Merging #4517 into master will increase coverage by 0.12%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4517      +/-   ##
==========================================
+ Coverage   56.01%   56.14%   +0.12%     
==========================================
  Files         185      185              
  Lines        6264     6282      +18     
  Branches        3        3              
==========================================
+ Hits         3509     3527      +18     
  Misses       2754     2754              
  Partials        1        1

Impacted Files	Coverage Δ
packages/jest-haste-map/src/worker.js	`93.75% <ø> (ø)`	⬆️
packages/jest-runner/src/run_test.js	`2.22% <ø> (ø)`	⬆️
packages/jest-docblock/src/index.js	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e3c423...12a25b5. Read the comment docs.

duailibe · 2017-09-20T12:57:53Z

packages/jest-docblock/README.md


 const docblock = extract(code);
 console.log(docblock); // "/**\n * Everything is awesome!\n * \n * @everything is:awesome\n * @flow\n */"

 const pragmas = parse(docblock);
 console.log(pragmas); // { everything: "is:awesome", flow: "" }
+
+console.log(print(pragmas), "hi!") // /**\n * hi!\n *\n * @everything is:awesome\n * @flow\n */;


Should be

console.log(print(pragmas, "hi!"))

samouri · 2017-09-20T15:10:57Z

@azz, this is awesome and will definitely help with implementing --prependPragma!

I have one question on it though. Wouldn't the prependPragma use-case also need to be able to extract the non-pragma portion of the docblock?

That way if the docblock in a file looks like this:

License 2017 (c) Something Inc.
@providesFruit tomatoes

we'd want to somehow be able to call:

docblock.print({ format: '', providesFruit: 'tomatoes' }, "License 2017 (c) Something Inc." );

Which would then output:

License 2017 (c) Something Inc.
@format
@providesFruit tomatoes

The options I can think of are either:

create a new exported function extractWithoutPragmas
make a breaking change to parse so that it instead returns { pragmas: {...}, comment: 'text' }

SimenB · 2017-09-20T15:16:57Z

packages/jest-docblock/src/index.js

@@ -8,6 +8,8 @@
 * @flow
 */

+const os = require('os');


should use import

SimenB · 2017-09-20T15:17:39Z

packages/jest-docblock/src/index.js

 exports.extract = extract;
 exports.parse = parse;
+exports.print = print;


these can probably be separate export functions. Not sure why I didn't convert them in my PR for ESM exports...

SimenB · 2017-09-20T15:19:09Z

packages/jest-docblock/src/index.js

+
+  const printedComments =
+    comments
+      .split(os.EOL)


can we detect the original EOL in the comment instead (https://www.npmjs.com/package/detect-newline)? People might use LF even if they're on Windows (e.g. airbnb eslint config requires LF regardless of OS)

Should I do the same for the existing parse, which has a hard-coded \n?

I think that makes sense, yeah 🙂

cpojer

See @SimenB's comments. I'll let this bake for a little longer until you guys figure out what the right way for this module is :)

azz · 2017-09-21T01:44:40Z

I've added a new function, parseWithComments with the signature:

parseWithComments(docblock: string): { comments: string, pragmas: {[key: string]: string} }

How does that sound?

Still WIP - need to address the EOL comments and add more test cases.

azz · 2017-09-21T12:27:59Z

packages/jest-docblock/src/index.js

 }

-exports.extract = extract;
-exports.parse = parse;
+export default {extract, parse, parseWithComments, print};


This was required for existing code that does import docblock from 'jest-docblock' to work without changes.

@SimenB should I leave this here or update the existing imports?

Either way works for me. @cpojer thoughts?

Fine to make a breaking change here, I'll publish 21.2 tomorrow or so, and for minor releases we can break the boundaries between Jest's modules.

samouri · 2017-09-21T20:43:48Z

The parseWithComments signature looks good! It even seems like it could be the default behavior for parse.

azz · 2017-09-21T23:20:38Z

Only reason I didn't do that is it would be a breaking change. @cpojer What do you prefer?

BREAKING CHANGE: Changes jest-docblock to an ES module without a default export, requires `import * as docblock from 'jest-docbloc'`

samouri · 2017-09-22T14:13:03Z

packages/jest-docblock/src/__tests__/index.test.js

+      os.EOL +
+      ' */';
+    expect(docblock.parseWithComments(code)).toEqual({
+      comments: 'hello' + os.EOL + os.EOL + 'world',


With respect to this case of:

/** * hello * @flow yes * * world */

I'd expect the result of parseWithComments to only have a single EOL between the hello and world like:

{ comments: 'hello' + os.EOL + 'world', pragmas { flow: 'yes' } }

I modified the test case here to show what I mean: https://github.com/azz/jest/pull/1/files

This was actually intended. Because we'll be reprinting comments above pragmas, it made sense to me that:

/** * My module does special snowflake * thing x really well. * @flow * * Copyright (c) 1234 Some Guy */

print(parseWithComments(text)) to reformat to:

/** * My module does special snowflake * thing x really well. * * Copyright (c) 1234 Some Guy * * @flow */

azz · 2017-09-27T10:57:51Z

Best merge conflict ever.

cpojer · 2017-09-28T14:22:18Z

Published jest@test for you guys to use this stuff in prettier.

* [jest-docblock]: add docblock.print() * Add missing flow type, clean up * Fix docs typo * Fix doc typo * [WIP] add docblock.parseWithComments() * Detect newline character, consolidate API * Fixup types * Remove default export from jest-docblock BREAKING CHANGE: Changes jest-docblock to an ES module without a default export, requires `import * as docblock from 'jest-docbloc'`

github-actions · 2021-05-13T06:06:43Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

[jest-docblock]: add docblock.print()

9828730

facebook-github-bot added the CLA Signed ✔️ label Sep 20, 2017

azz mentioned this pull request Sep 20, 2017

Add option to insert @format to first docblock if absent prettier/prettier#2865

Merged

azz added 2 commits September 20, 2017 22:41

Add missing flow type, clean up

17d8f0f

Fix docs typo

130628f

duailibe reviewed Sep 20, 2017

View reviewed changes

Fix doc typo

66fdd4a

SimenB reviewed Sep 20, 2017

View reviewed changes

packages/jest-docblock/src/index.js Outdated

@@ -8,6 +8,8 @@

* @flow

*/

const os = require('os');

Copy link

Member

SimenB Sep 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use import

SimenB reviewed Sep 20, 2017

View reviewed changes

[WIP] add docblock.parseWithComments()

c0b6a97

cpojer requested changes Sep 21, 2017

View reviewed changes

azz changed the title ~~[jest-docblock]: add docblock.print()~~ [WIP] [jest-docblock] add docblock.print() Sep 21, 2017

azz added 2 commits September 21, 2017 21:51

Detect newline character, consolidate API

854afa6

Fixup types

dc34f5d

azz changed the title ~~[WIP] [jest-docblock] add docblock.print()~~ [jest-docblock] add docblock.print() Sep 21, 2017

azz commented Sep 21, 2017

View reviewed changes

Remove default export from jest-docblock

a13f950

BREAKING CHANGE: Changes jest-docblock to an ES module without a default export, requires `import * as docblock from 'jest-docbloc'`

samouri reviewed Sep 22, 2017

View reviewed changes

Merge branch 'master' into docblock-print

12a25b5

SimenB approved these changes Sep 28, 2017

View reviewed changes

cpojer merged commit 80bd4cd into jestjs:master Sep 28, 2017

azz deleted the docblock-print branch September 30, 2017 02:35

azz mentioned this pull request Oct 1, 2017

[jest-docblock] Preserve leading whitespace in docblock comments #4576

Merged

renovate-tests mentioned this pull request Feb 6, 2018

Update dependency jest to v22 - autoclosed renovate-tests/changelog-md#3

Closed

renovate bot mentioned this pull request Feb 14, 2018

fix(deps): update dependency jest to ^22.0.0 manekinekko/devoxx-assistant#3

Open

github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jest-docblock] add docblock.print() #4517

[jest-docblock] add docblock.print() #4517

azz commented Sep 20, 2017 •

edited

Loading

cpojer commented Sep 20, 2017

codecov-io commented Sep 20, 2017 •

edited

Loading

duailibe Sep 20, 2017

samouri commented Sep 20, 2017

SimenB Sep 20, 2017

SimenB Sep 20, 2017

SimenB Sep 20, 2017 •

edited

Loading

azz Sep 21, 2017

SimenB Sep 21, 2017

cpojer left a comment

azz commented Sep 21, 2017 •

edited

Loading

azz Sep 21, 2017

SimenB Sep 22, 2017

cpojer Sep 22, 2017

samouri commented Sep 21, 2017 •

edited

Loading

azz commented Sep 21, 2017

samouri Sep 22, 2017 •

edited

Loading

samouri Sep 22, 2017

azz Sep 23, 2017 •

edited

Loading

azz commented Sep 27, 2017

cpojer commented Sep 28, 2017

github-actions bot commented May 13, 2021

[jest-docblock] add docblock.print() #4517

[jest-docblock] add docblock.print() #4517

Conversation

azz commented Sep 20, 2017 • edited Loading

cpojer commented Sep 20, 2017

codecov-io commented Sep 20, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

samouri commented Sep 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer left a comment

Choose a reason for hiding this comment

azz commented Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouri commented Sep 21, 2017 • edited Loading

azz commented Sep 21, 2017

samouri Sep 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azz Sep 23, 2017 • edited Loading

Choose a reason for hiding this comment

azz commented Sep 27, 2017

cpojer commented Sep 28, 2017

github-actions bot commented May 13, 2021

azz commented Sep 20, 2017 •

edited

Loading

codecov-io commented Sep 20, 2017 •

edited

Loading

SimenB Sep 20, 2017 •

edited

Loading

azz commented Sep 21, 2017 •

edited

Loading

samouri commented Sep 21, 2017 •

edited

Loading

samouri Sep 22, 2017 •

edited

Loading

azz Sep 23, 2017 •

edited

Loading