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

Fix validate error #214

Merged
merged 2 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/silent-steaks-chew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hypermod/validator': patch
---

Fixes usage of fetchConfig to account for undefined. Now errors should be reported correctly
5 changes: 5 additions & 0 deletions .changeset/slimy-mangos-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hypermod/fetcher': patch
---

Correctly types fetchConfig function to correctly show that this function can return `undefined`
5 changes: 5 additions & 0 deletions .changeset/wild-pigs-care.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hypermod/mod-atlaskit__textfield': patch
---

Removes unused variables
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ function transformer(
return source.toSource(options.printOptions);
}

const importToDoComment = `
/* TODO: (@hypermod) This file uses exports used to help theme @atlaskit/textfield which
has now been removed due to its poor performance characteristics. */`;

describe('Remove imports', () => {
it('should remove theme imports from Textfield and leave a comment', async () => {
const result = await applyTransform(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ function transformer(
return source.toSource(options.printOptions);
}

const themeToDoComment = `
/* TODO: (@hypermod) This file uses the @atlaskit/textfield \`theme\` prop which
has now been removed due to its poor performance characteristics. We have not replaced
theme with an equivalent API due to minimal usage of the \`theme\` prop.
The appearance of TextField will have likely changed. */`;

describe('Remove prop', () => {
it('should remove theme from Textfield and leave a comment', async () => {
const result = await applyTransform(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import core, { API, FileInfo, Options, Collection } from 'jscodeshift';

import { hasImportDeclaration, hasImportSpecifier } from '@hypermod/utils';
import { hasImportSpecifier } from '@hypermod/utils';

export default function transformer(
file: FileInfo,
Expand Down Expand Up @@ -102,7 +102,7 @@
path.node.callee.property.name === 'defineInlineTest'),
)
.forEach(path => {
const [transformerOpts, _configOpts, input, output, description] =

Check warning on line 105 in community/hypermod/src/defineInlineTest-to-applyTransform/transform.ts

View workflow job for this annotation

GitHub Actions / build (16.x)

'_configOpts' is assigned a value but never used
path.node.arguments;

const transformer = j(transformerOpts).find(j.ObjectProperty, {
Expand Down
40 changes: 22 additions & 18 deletions packages/fetcher/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ describe('fetcher', () => {
{ virtual: true },
);

const { filePath, config } = await fetchConfig(mockBasePath);
const configMeta = await fetchConfig(mockBasePath);

expect(config).toEqual(mockConfig);
expect(filePath).toEqual(mockFilePath);
expect(configMeta!.config).toEqual(mockConfig);
expect(configMeta!.filePath).toEqual(mockFilePath);
});

it('fetches config with named export', async () => {
Expand All @@ -57,10 +57,12 @@ describe('fetcher', () => {
},
);

const { filePath, config } = await fetchConfig(mockBasePath);
const configMeta = await fetchConfig(mockBasePath);

expect(config).toEqual(mockConfig);
expect(filePath).toEqual(path.join(mockBasePath, 'hypermod.config.js'));
expect(configMeta!.config).toEqual(mockConfig);
expect(configMeta!.filePath).toEqual(
path.join(mockBasePath, 'hypermod.config.js'),
);
});

it('fetches first matched config when multiple are found', async () => {
Expand All @@ -81,10 +83,10 @@ describe('fetcher', () => {
path.join(mockBasePath, 'codemods', 'hypermod.config.tsx'),
];

const { config, filePath } = await fetchConfig(mockBasePath);
const configMeta = await fetchConfig(mockBasePath);

expect(config).toEqual(mockConfig);
expect(filePath).toEqual(
expect(configMeta!.config).toEqual(mockConfig);
expect(configMeta!.filePath).toEqual(
path.join(mockBasePath, 'src', 'hypermod.config.ts'),
);
});
Expand All @@ -107,13 +109,13 @@ describe('fetcher', () => {
require: jest.fn().mockReturnValue(mockConfig),
};

const { filePath, config } = await fetchPackage(
const configMeta = await fetchPackage(
'fake-package',
mockPackageManager as unknown as PluginManager,
);

expect(config).toEqual(mockConfig);
expect(filePath).toEqual(mockFilePath);
expect(configMeta!.config).toEqual(mockConfig);
expect(configMeta!.filePath).toEqual(mockFilePath);
});

it('should throw if fetching fails', async () => {
Expand Down Expand Up @@ -146,13 +148,15 @@ describe('fetcher', () => {
}),
};

const { config, filePath } = await fetchRemotePackage(
const configMeta = await fetchRemotePackage(
'fake-package',
mockPackageManager as unknown as PluginManager,
);

expect(config).toEqual(mockConfig);
expect(filePath).toEqual(mockBasePath + '/hypermod.config.js');
expect(configMeta!.config).toEqual(mockConfig);
expect(configMeta!.filePath).toEqual(
mockBasePath + '/hypermod.config.js',
);
});

it('should throw if fetching fails', async () => {
Expand All @@ -179,13 +183,13 @@ describe('fetcher', () => {
}),
};

const { config, filePath } = await fetchRemotePackage(
const configMeta = await fetchRemotePackage(
'fake-package',
mockPackageManager as unknown as PluginManager,
);

expect(config).toEqual(mockConfig);
expect(filePath).toEqual(mockBasePath + '/index.js');
expect(configMeta!.config).toEqual(mockConfig);
expect(configMeta!.filePath).toEqual(mockBasePath + '/index.js');
});

it('throws if entrypoint-based config does not contain a valid config (and there are no config files available elsewhere)', async () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/fetcher/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ function requireConfig(filePath: string, resolvedPath: string) {
}
}

export async function fetchConfig(filePath: string): Promise<ConfigMeta> {
export async function fetchConfig(
filePath: string,
): Promise<ConfigMeta | undefined> {
const configs = await fetchConfigs(filePath);
return configs[0];
}
Expand Down Expand Up @@ -94,7 +96,7 @@ export async function fetchPackage(
export async function fetchRemotePackage(
packageName: string,
packageManager: PluginManager,
): Promise<ConfigMeta> {
): Promise<ConfigMeta | undefined> {
if (['javascript', 'typescript'].includes(packageName)) {
throw new Error(`'${packageName}' is ignored as a remote package.`);
}
Expand Down
10 changes: 10 additions & 0 deletions packages/validator/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ describe('validator', () => {
);
});

it('should error if config file is not found', async () => {
const filePath = 'path/to/';

(fetchConfig as jest.Mock).mockResolvedValue(undefined);

await expect(isValidConfigAtPath(filePath)).rejects.toThrowError(
`Unable to locate config file at path: ${filePath}`,
);
});

it('should error if config contains multiple invalid properties', async () => {
(fetchConfig as jest.Mock).mockResolvedValue({
location: 'path/to/hypermod.config.js',
Expand Down
10 changes: 5 additions & 5 deletions packages/validator/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,25 @@ export function isValidConfig(config: CodeshiftConfig) {
}

export async function isValidConfigAtPath(filePath: string) {
const { config } = await fetchConfig(filePath);
const configMeta = await fetchConfig(filePath);

if (!config) {
if (!configMeta) {
throw new Error(`Unable to locate config file at path: ${filePath}`);
}

const invalidProperites = getInvalidProperties(config);
const invalidProperites = getInvalidProperties(configMeta.config);
if (invalidProperites.length) {
throw new Error(
`Invalid transform ids found: ${invalidProperites.join(', ')}`,
);
}

if (!hasValidTransforms(config)) {
if (!hasValidTransforms(configMeta.config)) {
throw new Error(`Invalid transform ids found for config at "${filePath}".
Please make sure all transforms are identified by a valid semver version. ie 10.0.0`);
}

if (!hasValidPresets(config)) {
if (!hasValidPresets(configMeta.config)) {
throw new Error(`Invalid preset ids found for config at "${filePath}".
Please make sure all presets are kebab case and contain no spaces or special characters. ie sort-imports-by-scope`);
}
Expand Down
6 changes: 3 additions & 3 deletions scripts/docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ async function main() {
const directories = communityCodemods.filter(dir => junk.not(dir));

for (const dir of directories) {
const { config } = await fetchConfig(path.join(COMMUNITY_PATH, dir));
const configMeta = await fetchConfig(path.join(COMMUNITY_PATH, dir));

if (!config) {
if (!configMeta || !configMeta.config) {
throw new Error(`Unable to locate config for path: ${dir}`);
}

data.push({ name: dir, config });
data.push({ name: dir, config: configMeta.config });
}

cleanTargetDir(DOCS_PATH);
Expand Down
Loading