From 98e2ea90f4b3c63055139e2759d388c653ca16a6 Mon Sep 17 00:00:00 2001 From: Viktor Charypar Date: Mon, 27 Mar 2017 21:43:28 +0100 Subject: [PATCH] feat(useLocalPrettier): Prefer local prettier over bundled To make prettier-atom work nicer in projects with prettier installed, it will now prefer the local version of the prettier package over the bundled dependency, to give output consistent with package scripts and CLI prettier. If no local prettier can be found, we fallback to the bundled package. --- dist/executePrettier.js | 22 ++++++++++++++++++++- dist/helpers.js | 11 +++++++++++ src/executePrettier.js | 20 ++++++++++++++++++- src/executePrettier.test.js | 13 +++++++++++++ src/helpers.js | 11 +++++++++++ src/helpers.test.js | 38 +++++++++++++++++++++++++++++++++++++ tests/fixtures/prettier.js | 6 ++++++ 7 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/prettier.js diff --git a/dist/executePrettier.js b/dist/executePrettier.js index 706a4b80..7e890731 100644 --- a/dist/executePrettier.js +++ b/dist/executePrettier.js @@ -12,6 +12,7 @@ var _require2 = require('./helpers'), getPrettierOptions = _require2.getPrettierOptions, getPrettierEslintOptions = _require2.getPrettierEslintOptions, getCurrentFilePath = _require2.getCurrentFilePath, + getLocalPrettierPath = _require2.getLocalPrettierPath, shouldDisplayErrors = _require2.shouldDisplayErrors, shouldUseEslint = _require2.shouldUseEslint, runLinter = _require2.runLinter; @@ -31,6 +32,17 @@ var handleError = function handleError(error) { return false; }; +// charypar: This is currently the best way to use local prettier instance. +// Using the CLI introduces a noticeable delay and there is currently no +// way to use prettier as a long-running process for formatting files as needed +// +// See https://github.com/prettier/prettier/issues/918 +// +// $FlowFixMe when possible, don't use dynamic require +var getLocalPrettier = function getLocalPrettier(path) { + return require(path); +}; // eslint-disable-line + var executePrettier = function executePrettier(editor, text) { try { if (shouldUseEslint()) { @@ -41,7 +53,15 @@ var executePrettier = function executePrettier(editor, text) { })); }); } - return prettier.format(text, getPrettierOptions(editor)); + + var prettierOptions = getPrettierOptions(editor); + var localPrettier = getLocalPrettierPath(getCurrentFilePath(editor)); + + if (!localPrettier) { + return prettier.format(text, prettierOptions); + } + + return getLocalPrettier(localPrettier).format(text, prettierOptions); } catch (error) { return handleError(error); } diff --git a/dist/helpers.js b/dist/helpers.js index 86585f9c..837f6b76 100644 --- a/dist/helpers.js +++ b/dist/helpers.js @@ -37,6 +37,16 @@ var getNearestEslintignorePath = function getNearestEslintignorePath(filePath) { return findCached(getDirFromFilePath(filePath), '.eslintignore'); }; +var getLocalPrettierPath = function getLocalPrettierPath(filePath) { + if (!filePath) return null; + + var indexPath = path.join('node_modules', 'prettier', 'index.js'); + var dirPath = getDirFromFilePath(filePath); + if (!dirPath) return null; + + return findCached(dirPath, indexPath); +}; + var getFilePathRelativeToEslintignore = function getFilePathRelativeToEslintignore(filePath) { var nearestEslintignorePath = getNearestEslintignorePath(filePath); @@ -168,6 +178,7 @@ module.exports = { getPrettierOption: getPrettierOption, getPrettierEslintOption: getPrettierEslintOption, getCurrentFilePath: getCurrentFilePath, + getLocalPrettierPath: getLocalPrettierPath, isInScope: isInScope, isCurrentScopeEmbeddedScope: isCurrentScopeEmbeddedScope, isFilePathEslintignored: isFilePathEslintignored, diff --git a/src/executePrettier.js b/src/executePrettier.js index 1b730e30..da462762 100644 --- a/src/executePrettier.js +++ b/src/executePrettier.js @@ -7,6 +7,7 @@ const { getPrettierOptions, getPrettierEslintOptions, getCurrentFilePath, + getLocalPrettierPath, shouldDisplayErrors, shouldUseEslint, runLinter, @@ -27,6 +28,15 @@ const handleError = (error) => { return false; }; +// charypar: This is currently the best way to use local prettier instance. +// Using the CLI introduces a noticeable delay and there is currently no +// way to use prettier as a long-running process for formatting files as needed +// +// See https://github.com/prettier/prettier/issues/918 +// +// $FlowFixMe when possible, don't use dynamic require +const getLocalPrettier = path => require(path); // eslint-disable-line + const executePrettier = (editor, text) => { try { if (shouldUseEslint()) { @@ -37,7 +47,15 @@ const executePrettier = (editor, text) => { filePath: getCurrentFilePath(editor), })); } - return prettier.format(text, getPrettierOptions(editor)); + + const prettierOptions = getPrettierOptions(editor); + const localPrettier = getLocalPrettierPath(getCurrentFilePath(editor)); + + if (!localPrettier) { + return prettier.format(text, prettierOptions); + } + + return getLocalPrettier(localPrettier).format(text, prettierOptions); } catch (error) { return handleError(error); } diff --git a/src/executePrettier.test.js b/src/executePrettier.test.js index c7cb8b47..0c8b18ff 100644 --- a/src/executePrettier.test.js +++ b/src/executePrettier.test.js @@ -1,4 +1,5 @@ // @flow +const path = require('path'); const prettier = require('prettier'); const prettierEslint = require('prettier-eslint'); @@ -95,6 +96,18 @@ describe('executePrettierOnBufferRange()', () => { }); }); + describe('when there is a local prettier', () => { + test('transforms using local prettier', () => { + const mockPrettierPath = path.join(__dirname, '..', 'tests', 'fixtures', 'prettier.js'); + // $FlowFixMe + helpers.getLocalPrettierPath.mockImplementation(() => mockPrettierPath); + + executePrettierOnBufferRange(editor, bufferRangeFixture); + + expect(editor.setTextInBufferRange).toHaveBeenCalledWith(bufferRangeFixture, 'mock formatted text'); + }); + }); + describe('when prettier throws an error', () => { beforeEach(() => { prettier.format.mockImplementation(() => { diff --git a/src/helpers.js b/src/helpers.js index 7feaf666..5816389f 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -22,6 +22,16 @@ const getDirFromFilePath = (filePath: FilePath): FilePath => path.parse(filePath const getNearestEslintignorePath = (filePath: FilePath): ?FilePath => findCached(getDirFromFilePath(filePath), '.eslintignore'); +const getLocalPrettierPath = (filePath: ?FilePath): ?FilePath => { + if (!filePath) return null; + + const indexPath = path.join('node_modules', 'prettier', 'index.js'); + const dirPath = getDirFromFilePath(filePath); + if (!dirPath) return null; + + return findCached(dirPath, indexPath); +}; + const getFilePathRelativeToEslintignore = (filePath: FilePath): ?FilePath => { const nearestEslintignorePath = getNearestEslintignorePath(filePath); @@ -122,6 +132,7 @@ module.exports = { getPrettierOption, getPrettierEslintOption, getCurrentFilePath, + getLocalPrettierPath, isInScope, isCurrentScopeEmbeddedScope, isFilePathEslintignored, diff --git a/src/helpers.test.js b/src/helpers.test.js index b45c70b5..9fa0f92e 100644 --- a/src/helpers.test.js +++ b/src/helpers.test.js @@ -9,6 +9,7 @@ const { getPrettierOption, getPrettierEslintOption, getCurrentFilePath, + getLocalPrettierPath, isInScope, isFilePathEslintignored, isFilePathExcluded, @@ -96,6 +97,43 @@ describe('getCurrentFilePath', () => { }); }); +describe('getLocalPrettierPath', () => { + test('is null if no prettier package can be found', () => { + atomLinter.findCached.mockImplementation(() => undefined); + const filePath = path.join(__dirname, 'sourceFile.js'); + + const actual = getLocalPrettierPath(filePath); + const expected = undefined; + + expect(actual).toEqual(expected); + }); + + test('is prettier instance if one could be found', () => { + const prettierLib = path.join(__dirname, '..', 'node_modules', 'prettier', 'index.js'); + + atomLinter.findCached.mockImplementation(() => prettierLib); + const filePath = path.join(__dirname, '..', 'tests', 'fixtures', 'sourceFile.js'); + + const actual = getLocalPrettierPath(filePath); + const expected = prettierLib; + + expect(actual).toBe(expected); + }); + + test('looks for prettier entry file', () => { + const prettierLib = path.join(__dirname, '..', 'node_modules', 'prettier', 'index.js'); + + atomLinter.findCached.mockImplementation(() => prettierLib); + const filePath = path.join(__dirname, '..', 'tests', 'fixtures', 'sourceFile.js'); + + getLocalPrettierPath(filePath); + const expectedDir = path.join(__dirname, '..', 'tests', 'fixtures'); + const expectedLib = path.join('node_modules', 'prettier', 'index.js'); + + expect(atomLinter.findCached).toHaveBeenCalledWith(expectedDir, expectedLib); + }); +}); + describe('isInScope', () => { test("returns true if the editor's current scope is listed among the config's scopes", () => { const mockGet = jest.fn(() => ['source.js.jsx', 'text.html.vue']); diff --git a/tests/fixtures/prettier.js b/tests/fixtures/prettier.js new file mode 100644 index 00000000..bb20270d --- /dev/null +++ b/tests/fixtures/prettier.js @@ -0,0 +1,6 @@ + +module.exports = { + format() { + return 'mock formatted text'; + }, +};