Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use correct parser plugins for TS and Vue #91

Merged
merged 7 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/preprocessors/preprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getExperimentalParserPlugins } from '../utils/get-experimental-parser-p
import { getSortedNodes } from '../utils/get-sorted-nodes';

export function preprocessor(code: string, options: PrettierOptions): string {
const { importOrderParserPlugins, importOrder } = options;
const { importOrderParserPlugins, importOrder, filepath } = options;
let { importOrderTypeScriptVersion } = options;
const isTSSemverValid = semver.valid(importOrderTypeScriptVersion);

Expand All @@ -29,10 +29,15 @@ export function preprocessor(code: string, options: PrettierOptions): string {
: true;

const allOriginalImportNodes: ImportDeclaration[] = [];
let plugins = getExperimentalParserPlugins(importOrderParserPlugins);
// Do not inject jsx plugin for non-jsx ts files
if (filepath.endsWith('.ts')) {
plugins = plugins.filter((p) => p !== 'jsx');
fbartho marked this conversation as resolved.
Show resolved Hide resolved
}
const parserOptions: ParserOptions = {
sourceType: 'module',
attachComment: true,
plugins: getExperimentalParserPlugins(importOrderParserPlugins),
plugins,
};

// Disable importOrderCombineTypeAndValueImports if typescript is not set to a version that supports it
Expand Down
75 changes: 67 additions & 8 deletions src/preprocessors/vue.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type { parse as Parse } from '@vue/compiler-sfc';

import { ImportOrderParserPlugin } from '../../types';
import { PrettierOptions } from '../types';
import { hasPlugin } from '../utils/get-experimental-parser-plugins';
import { preprocessor } from './preprocessor';

export function vuePreprocessor(code: string, options: PrettierOptions) {
Expand All @@ -10,18 +12,18 @@ export function vuePreprocessor(code: string, options: PrettierOptions) {
const { descriptor } = parse(code);

if (descriptor.script) {
const { content } = descriptor.script;
preprocessedCode = preprocessedCode.replace(
content,
`\n${preprocessor(content, options)}\n`,
preprocessedCode = sortScript(
descriptor.script,
preprocessedCode,
options,
);
}

if (descriptor.scriptSetup) {
const { content } = descriptor.scriptSetup;
preprocessedCode = preprocessedCode.replace(
content,
`\n${preprocessor(content, options)}\n`,
preprocessedCode = sortScript(
descriptor.scriptSetup,
preprocessedCode,
options,
);
}

Expand All @@ -35,3 +37,60 @@ export function vuePreprocessor(code: string, options: PrettierOptions) {
}
}
}

function isTS(lang?: string) {
return lang === 'ts' || lang === 'tsx';
}

/**
* Configures correct babel plugins, sorts imports in a script or setupScript,
* and replaces that script/setupScript within the original code
*
* Much of this was adapted from https://github.com/vuejs/vue/blob/49b6bd4264c25ea41408f066a1835f38bf6fe9f1/packages/compiler-sfc/src/compileScript.ts#L118-L134
*
* @param param0 a script or setupScript
* @param code Source code of the file
* @param options Prettier options
* @returns Original code with sorted imports in the script provided
*/
function sortScript(
{ content, lang }: { content: string; lang?: string },
code: string,
options: PrettierOptions,
) {
const { importOrderParserPlugins = [] } = options;
let pluginClone = [...importOrderParserPlugins];
const newPlugins: ImportOrderParserPlugin[] = [];

if (!isTS(lang) || lang === 'tsx') {
newPlugins.push('jsx');
} else {
// Remove jsx if typescript and not tsx
pluginClone = pluginClone.filter((p) => p !== 'jsx');
}

newPlugins.push(...pluginClone);

if (isTS(lang)) {
if (!hasPlugin(newPlugins, 'typescript')) {
newPlugins.push('typescript');
}

if (
!hasPlugin(newPlugins, 'decorators') &&
!hasPlugin(newPlugins, 'decorators-legacy')
) {
newPlugins.push('decorators-legacy');
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m uncomfortable with injecting decorators, because I’ve seen people fork every plugin. I worry that we’ll always be injecting decorators-legacy.

I’d be less uncomfortable if you were to shift this logic into the conditional-injection of the ‘typescript’ plugin. (Move the latter if-block to line 77).

I’d be least uncomfortable if this logic only happened if users omitted importOrderParserPlugins entirely. Then we definitely have permission to do automatic behavior. — If users are passing us plugins, I worry that manipulating it could only lead to more, painful errors.

Copy link
Owner Author

Choose a reason for hiding this comment

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

One problem we have is that we always inject typescript and jsx, and there's no way to know whether it was us or the user who did it. I'd like to get to the point where we can remove those defaults and be smart about when we default to various plugins, but I'm not all the way there yet. I'll see what it would take to do that as part of 4.0.

As for the decorators specifically, that was something I copied straight from vue (see vuejs/vue@326d24a). I'm not a vue user, so I'm not sure why it's important to include a decorator plugin, but I think it might be related to the older class-based component definition in Vue 2: https://v2.vuejs.org/v2/guide/typescript.html#Class-Style-Vue-Components.

So, I'm happy to remove the injection of the decorators plugin. Are you okay with the rest of it staying as-is?


const adjustedOptions = {
...options,
importOrderParserPlugins: newPlugins,
};

return code.replace(
content,
`\n${preprocessor(content, adjustedOptions)}\n`,
);
}
32 changes: 32 additions & 0 deletions src/utils/get-experimental-parser-plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,35 @@ export const getExperimentalParserPlugins = (
return plugin;
});
};

/**
* Checks whether a specified plugin is included in importOrderParserPlugins.
* More fancy than just a `.includes()` because importOrderParserPlugins can contain plugins with configuration
*
* @param importOrderParserPlugins array of experimental babel parser plugins
* @returns true if the plugin is in the list
*/
export const hasPlugin = (
importOrderParserPlugins: string[],
pluginName: string,
): boolean => {
for (const pluginNameOrJson of importOrderParserPlugins) {
const isParserPluginWithOptions = pluginNameOrJson.startsWith('[');
let plugin;

if (isParserPluginWithOptions) {
try {
plugin = JSON.parse(pluginNameOrJson)[0];
} catch (e) {
throw Error(
'Invalid JSON in importOrderParserPlugins: ' +
pluginNameOrJson,
);
}
} else {
plugin = pluginNameOrJson as ParserPlugin;
}
if (plugin === pluginName) return true;
}
return false;
};
52 changes: 52 additions & 0 deletions tests/Typescript/__snapshots__/ppsi.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,55 @@ export class AppComponent {
}

`;

exports[`jsx.tsx - typescript-verify > jsx.tsx 1`] = `
import z from 'z';
import { isEmpty } from "lodash-es";
import threeLevelRelativePath from "../../../threeLevelRelativePath";
import sameLevelRelativePath from "./sameLevelRelativePath";
import thirdParty from "third-party";
import oneLevelRelativePath from "../oneLevelRelativePath";
import otherthing from "@core/otherthing";
import abc from "@core/abc";
import twoLevelRelativePath from "../../twoLevelRelativePath";
import component from "@ui/hello";
import fourLevelRelativePath from "../../../../fourLevelRelativePath";
import something from "@server/something";
import xyz from "@ui/xyz";

export const Component = () => {
return <div>Component</div>
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
import { isEmpty } from "lodash-es";
import thirdParty from "third-party";
import z from "z";
import abc from "@core/abc";
import otherthing from "@core/otherthing";
import something from "@server/something";
import component from "@ui/hello";
import xyz from "@ui/xyz";
import fourLevelRelativePath from "../../../../fourLevelRelativePath";
import threeLevelRelativePath from "../../../threeLevelRelativePath";
import twoLevelRelativePath from "../../twoLevelRelativePath";
import oneLevelRelativePath from "../oneLevelRelativePath";
import sameLevelRelativePath from "./sameLevelRelativePath";

export const Component = () => {
return <div>Component</div>;
};

`;

exports[`old-style-assertion.ts - typescript-verify > old-style-assertion.ts 1`] = `
import { b } from "b";
import { a } from "a";

<A>a();
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
import { a } from "a";
import { b } from "b";

<A>a();

`;
17 changes: 17 additions & 0 deletions tests/Typescript/jsx.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import z from 'z';
import { isEmpty } from "lodash-es";
import threeLevelRelativePath from "../../../threeLevelRelativePath";
import sameLevelRelativePath from "./sameLevelRelativePath";
import thirdParty from "third-party";
import oneLevelRelativePath from "../oneLevelRelativePath";
import otherthing from "@core/otherthing";
import abc from "@core/abc";
import twoLevelRelativePath from "../../twoLevelRelativePath";
import component from "@ui/hello";
import fourLevelRelativePath from "../../../../fourLevelRelativePath";
import something from "@server/something";
import xyz from "@ui/xyz";

export const Component = () => {
return <div>Component</div>
}
4 changes: 4 additions & 0 deletions tests/Typescript/old-style-assertion.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { b } from "b";
import { a } from "a";

<A>a();
2 changes: 1 addition & 1 deletion tests/Typescript/ppsi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import {run_spec} from '../../test-setup/run_spec';

run_spec(__dirname, ["typescript"], {
importOrder: ['^@core/(.*)$', '^@server/(.*)', '^@ui/(.*)$', '^[./]'],
importOrderParserPlugins : ['typescript', 'decorators-legacy', 'classProperties'],
importOrderParserPlugins : ['typescript', 'jsx', 'decorators-legacy', 'classProperties'],
});
35 changes: 35 additions & 0 deletions tests/Vue/__snapshots__/ppsi.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,40 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`jsx-in-ts.vue - vue-verify > jsx-in-ts.vue 1`] = `
<template>
<router-view />
</template>

<script lang="ts">
// This will crash if we try to use the \`jsx\` parser plugin
<A>a();
</script>

<style lang="less">
#app {
height: 100%;
background-color: inherit;
}
</style>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<template>
<router-view />
</template>

<script lang="ts">
// This will crash if we try to use the \`jsx\` parser plugin
<A>a();
</script>

<style lang="less">
#app {
height: 100%;
background-color: inherit;
}
</style>

`;

exports[`no-script.vue - vue-verify > no-script.vue 1`] = `
<template>
<router-view />
Expand Down
15 changes: 15 additions & 0 deletions tests/Vue/jsx-in-ts.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<template>
<router-view />
</template>

<script lang="ts">
// This will crash if we try to use the `jsx` parser plugin
<A>a();
</script>

<style lang="less">
#app {
height: 100%;
background-color: inherit;
}
</style>