prettier / prettier-atom

An atom package for the prettier formatter.
MIT License
754 stars 95 forks source link

RangeError: path should be a `path.relative()`d string when saving the user stylesheet (`styles.less`) in Atom with Prettier on Save enabled #473

Closed sompylasar closed 5 years ago

sompylasar commented 5 years ago

When saving the user stylesheet (styles.less) in Atom with Prettier on Save enabled, this error pops up:

RangeError: path should be a `path.relative()`d string, but got "/Users/sompylasar/.atom/styles.less"
    at throwError (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/ignore/index.js:354:9)
    at checkPath (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/ignore/index.js:375:12)
    at Ignore._test (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/ignore/index.js:483:5)
    at Ignore.ignores (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/ignore/index.js:518:17)
    at someGlobsMatchFilePath (/Users/sompylasar/.atom/packages/prettier-atom/dist/helpers/general.js:10:96)
    at _.flow.filePath (/Users/sompylasar/.atom/packages/prettier-atom/dist/formatOnSave/shouldFormatOnSave.js:30:101)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:49:144
    at n (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:89)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:51:79
    at h (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:7:166)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:51:56
    at n (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:89)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:63:257
    at n (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:89)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:51:79
    at u (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:521)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:51:56
    at n (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:89)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:63:257
    at n (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:89)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:97:406
    at n (/Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:89)
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:63:257
    at /Users/sompylasar/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:49:144
    at attemptWithErrorNotification (/Users/sompylasar/.atom/packages/prettier-atom/dist/formatOnSave/index.js:14:84)
    at /Users/sompylasar/.atom/packages/prettier-atom/dist/atomInterface/index.js:59:13
    at Generator.next (<anonymous>)
    at step (/Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
    at /Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/babel-runtime/helpers/asyncToGenerator.js:35:14
    at new Promise (<anonymous>)
    at new F (/Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/core-js/library/modules/_export.js:36:28)
    at /Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/babel-runtime/helpers/asyncToGenerator.js:14:12
    at attemptWithErrorNotification (/Users/sompylasar/.atom/packages/prettier-atom/dist/atomInterface/index.js:67:17)
    at safeFormatOnSaveIfAppropriate (/Users/sompylasar/.atom/packages/prettier-atom/dist/formatOnSave/index.js:14:49)
    at /Users/sompylasar/.atom/packages/prettier-atom/dist/main.js:38:23
    at Generator.next (<anonymous>)
    at step (/Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
    at /Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/babel-runtime/helpers/asyncToGenerator.js:35:14
    at new Promise (<anonymous>)
    at new F (/Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/core-js/library/modules/_export.js:36:28)
    at /Users/sompylasar/.atom/packages/autocomplete-flow/node_modules/babel-runtime/helpers/asyncToGenerator.js:14:12
    at lazyFormatOnSave (/Users/sompylasar/.atom/packages/prettier-atom/dist/main.js:42:17)
    at subscriptions.add.editor.getBuffer.onWillSave (/Users/sompylasar/.atom/packages/prettier-atom/dist/main.js:111:119)
    at Function.simpleDispatch (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11:1190583)
    at /Applications/Atom.app/Contents/Resources/app/static/<embedded>:11:1192208
    at Array.map (<anonymous>)
    at Emitter.emitAsync (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11:1192171)
    at TextBuffer.saveTo (/Applications/Atom.app/Contents/Resources/app/static/<embedded>:11:496823)

Prettier: Debug

Atom version: 1.33.0
prettier-atom version: 0.56.0
prettier: bundled
prettier version: 1.15.2
prettier-eslint version: 8.8.2
prettier-atom configuration: {
  "formatOnSaveOptions": {
    "enabled": true,
    "excludedGlobs": [
      "*.json"
    ],
    "jsonScopes": [
      "source.json-"
    ],
    "showInStatusBar": true,
    "respectEslintignore": true,
    "whitelistedGlobs": [],
    "isDisabledIfNotInPackageJson": false,
    "isDisabledIfNoConfigFile": false
  },
  "prettierOptions": {
    "printWidth": 120,
    "singleQuote": true,
    "trailingComma": "all"
  },
  "useEslint": false,
  "useStylelint": false,
  "prettierEslintOptions": {
    "prettierLast": false
  }
}
sompylasar commented 5 years ago

This may indicate a broader issue with some paths passed to the ignore package by prettier-atom.

robwise commented 5 years ago

Yeah unfortunately they changed their API to be very strict. We use Atom's relative feature to relative the path of the file you are formatting to the current Atom project's path, but it seems in certain cases, Atom is returning an absolute path (perhaps when there is no Atom project context present?).

I need to figure out a good fallback solution here. Will fix.

sompylasar commented 5 years ago

Thanks for a quick turnaround @robwise! Yes I think there can be no particular project context for the Atom's global configuration files. Why is relative path so important? I think if you run prettier-atom on an absolute path, it should use the same behavior like when it uses the global version of prettier and all other options like if there's no prettier configuration in the project (also consider not running it if "only if config is present" flag is set).

sompylasar commented 5 years ago

I also noticed that the status-bar Prettier on Save item is not present for these global files, this may be relevant, too, re: having a project context.

robwise commented 5 years ago

Why is relative path so important?

Haha, I had this same question, it's just some sort of breaking change that the ignore package introduced recently. Very annoying.

It's coming into play because we allow you to whitelist or blacklist files from prettier-atom using globs, and from the ignore package's point of view, the globs should always be matched against the file relative to the location of the ignore file, but we don't really have an ignore file because the globs are in Atom.

robwise commented 5 years ago

I also noticed that the status-bar Prettier on Save item is not present for these global files, this may be relevant, too, re: having a project context.

This is more due to the fact that Prettier now has this beta "plugins" feature where you may or may not be able to format a file depending on what plugins you have loaded.

There's no way for prettier-atom to know, therefore, whether you can format a file without going and asking Prettier.

Prettier requires us to pass the file itself in order for it to respond. Since we can't pass an unsaved file, we aren't able to tell that the file is formattable or not by Prettier.

sompylasar commented 5 years ago

Since we can't pass an unsaved file, we aren't able to tell that the file is formattable or not by Prettier.

In my case it was an existing file in the filesystem (the Atom config file).

What's the harm of showing the status-bar element at all times the prettier-atom package is enabled in Atom?

sompylasar commented 5 years ago

RE: ignore, they recommend doing path.relative('/', yourPath) if yourPath is absolute. Maybe prettier-atom should detect that the current path is outside the project directory, convert it to absolute and then to fsroot-relative prior to calling ignore?

robwise commented 5 years ago

In my case it was an existing file in the filesystem (the Atom config file).

Oh then that's just because Prettier can't infer a parser for CSON.

What's the harm of showing the status-bar element at all times the prettier-atom package is enabled in Atom?

We used to do this, but we got issues from people who found this annoying because they spent most of their time developing in languages like Python or something that aren't prettier-formattable files and didn't want what they considered an irrelevant plugin cluttering up their interface.

I suppose we could make it a prettier-atom option to always show it or hide for files where the parser cannot be inferred?

RE: ignore, they recommend doing path.relative('/', yourPath) if yourPath is absolute.

Yeah, the problem is that I was using atom.project.relativizePath to do this, but unknown to me at the time, if you don't have a discernable Atom project (such as when editing your Atom config.cson), it just returns an absolute path still!

Maybe prettier-atom should detect that the current path is outside the project directory, convert it to absolute and then to fsroot-relative prior to calling ignore?

Yep we came to close to the same conclusion lol, I just finished writing a fix that just relativizes to the parent directory. So if your file is /Users/rob/.atom/config.cson and there's no project, it resolves to config.cson for the ignore glob checks.

sompylasar commented 5 years ago

I suppose we could make it a prettier-atom option to always show it or hide for files where the parser cannot be inferred?

No, if the parser cannot be inferred just because the syntax is not supported, it's better to hide it. I was wondering if it's not visible for some other reason. I guess it's questionable whether to show if the syntax is supported but prettier for it is somehow disabled by the user. Maybe consider an icon instead of a textual label (if enabled, a combined icon with a tick; if disabled, a crossed out icon with a tooltip why disabled).

robwise commented 5 years ago

I think that's basically what we have now isn't it?

sompylasar commented 5 years ago

I think that's basically what we have now isn't it?

Yeah looks like, except having a smaller icon to not clutter the status-bar (but that's debatable what's better UX, it's not just an icon, it's a click area that's being used quite often if I switch between projects with and without Prettier formatting). Just checked the LESS stylesheet and it shows Prettier while CSON is not handled, so it's fine.