filepath serializer #3007
filepath serializer #3007
Conversation
jamiebuilds
force-pushed
the
filepath-serializer
branch
from
f1dfb9a to
43970d3
Compare
Codecov Report
@@ Coverage Diff @@
## master #3007 +/- ##
=========================================
- Coverage 68.49% 67.79% -0.7%
=========================================
Files 146 148 +2
Lines 5335 5354 +19
=========================================
- Hits 3654 3630 -24
- Misses 1681 1724 +43
Continue to review full report at Codecov.
|
| */ | ||
| 'use strict'; | ||
|
|
||
| const cwd = process.cwd(); |
There was a problem hiding this comment.
This is pretty cool but I don't think using process.cwd() is sufficient. The directory can change based on where Jest is invoked which would lead to outdated snapshots all the time. I'm wondering if we could make it configurable somehow?
There was a problem hiding this comment.
Alternatively it can search upwards until it finds a package.json#jest
packages/pretty-format/src/index.js
Outdated
| const plugin = printPlugin(val, indent, prevIndent, spacing, edgeSpacing, refs, maxDepth, currentDepth, plugins, min, callToJSON, printFunctionName, escapeRegex, colors); | ||
| if (plugin) { | ||
| return plugin; | ||
| } | ||
|
|
||
| const basic = printBasicValue(val, printFunctionName, escapeRegex); |
There was a problem hiding this comment.
What does this change do to performance?
There was a problem hiding this comment.
Ran the perf suite, didn't notice any significant differences anywhere.
There was a problem hiding this comment.
That's not the only way to specify a jest config, unfortunately. Would you mind divorcing the two changes you are making in this diff and sending a separate PR for the filepath thing? Thanks!
There was a problem hiding this comment.
Did you reply to the wrong thread?
There was a problem hiding this comment.
Oh, oops this was supposed to be a response to the filename serializer plugin.
jamiebuilds
force-pushed
the
filepath-serializer
branch
from
43970d3 to
f54517f
Compare
jamiebuilds
changed the title
|
Can this |
|
Thanks @thejameskyle. I think we can only make this work if we find a way to pass however, we don't have the |
|
We can allow serializer configuration by changing the API to: export default function({ rootDir }) {
return {
test() {...},
print() {...},
};
} |
|
Not a huge fan of breaking this but I guess we could keep backwards compatibility. With your proposal, we could thread some Jest config options through to pretty-format which then gets passed to plugins. I like it! |
This reverts commit 59b70c2. Needs to wait for normalizing of path jestjs/jest#3007 https://travis-ci.org/SimenB/csslint-stylish/builds/207717253
|
@thejameskyle are you planning on working on a way to pass the rootDir in? |
|
In a rush to finish some other things this week. But we'll need to agree on an API first. This is what I'm going with: type Printer = {
test(mixed): boolean,
print(mixed, Print, Indent, Options, Colors): string,
};
type PrinterBuilder = ({ rootDir: string }) => Printer;
type Plugin = Printer | PrinterBuilder; |
|
sounds good. Can we just call it "root"? I really dislike "rootDir" within Jest :) We'll also need to find a way to thread this through; tests don't expose the config/rootDir at the moment and I'd like to keep it that way. |
|
Happy to reopen if we decide to ship this inside of Jest with the iterated version, closing for now. |
|
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. |
jest-serializer-filepathcan be installed to replace local filepaths in strings to something that isn't specific to the cwd (/Users/thejameskyle/path/to/project/fixtures/file.js=>/../fixtures/file.js)