test: migrate plugin-resolution tests#42
Conversation
|
#41 got merged, can you rebase and check what else is needed? |
7abb00e to
479f692
Compare
|
@fabiospampinato this is blocked by #50 |
|
|
||
| // TODO (43081j): we don't currently support loading relative paths without | ||
| // the leading `./` | ||
| describe.skip("loads --plugin by filename without leading ./, should resolve to file, not package", () => { |
There was a problem hiding this comment.
note these skipped tests too
prettier somehow allows foo/bar/baz.js to behave as if it was ./foo/bar/baz.js
no clue how it does this without just trying twice (once for a relative path, once for a node path, because it could also be from the package foo)
There was a problem hiding this comment.
There was a problem hiding this comment.
aha thank you. so it is what i expected
the question is, do we want this in the new CLI? its easy to add but maybe it'd be a good time to get rid of these relative-but-not-really-relative paths? and just tell people to use actual paths, like ./foo
There was a problem hiding this comment.
Actually, I like how it works. Imagine if we support ignore files in config.
export default {
ignore: [
- "foo.js",
+ "./foo.js"
]
}No body want this.
Same here
export default {
plugins: [
- "node_modules/prettier-plugin-foo/index.js",
+ "./node_modules/prettier-plugin-foo/index.js"
]
}much easier to use.
Maybe someday, we should ask user to import plugin like ESLint did.
There was a problem hiding this comment.
Add the plugin path also not related to config file, but related to CWD, makes no sense to put ./ there.
At least it's how it works now, in v3 I actually try to change it, but turns out it's harder than I thought, so I gave up.
There was a problem hiding this comment.
Forgot details, but tricky part is here https://github.com/prettier/prettier/blob/af6e7215ce0e0d243cb34a85174af65ab4177f47/src/config/resolve-config.js#L67
There was a problem hiding this comment.
Add the plugin path also not related to config file, but related to CWD, makes no sense to put ./ there.
@fisker wait, are you saying the plugin path should be resolved related to the CWD instead of the folder where the config is found in? Because I don't see v3 actually working like that.
34e0257 to
f8dc396
Compare
|
#50 got fixed, do we need more changes to this PR before merging? |
Copies the `plugin-resolution` tests from prettier.
479f692 to
beb5749
Compare
- We currently don't support loading plugins with implicit relative paths (missing starting dot)
Copies the
plugin-resolutiontests from prettier.Blocked by #41