-
Notifications
You must be signed in to change notification settings - Fork 66
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: valid package json on app create #1031
Changes from 5 commits
75de26c
8a5671d
52b9ca1
6b75a18
1111bc5
930eadc
c5682da
b4d6a8d
03f40f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ import * as fs from 'fs-jetpack' | |
import { Command } from '../../lib/cli' | ||
import * as Layout from '../../lib/layout' | ||
import { rootLogger } from '../../lib/nexus-logger' | ||
import { CWDProjectNameOrGenerate, generateProjectName } from '../../lib/utils' | ||
import { casesHandled, CWDProjectNameOrGenerate, generateProjectName } from '../../lib/utils' | ||
import { run as runCreateApp } from './create/app' | ||
import { Dev } from './dev' | ||
|
||
|
@@ -72,6 +72,12 @@ export class __Default implements Command { | |
console.log() // space after codeblock | ||
|
||
break | ||
case 'malformed_package_json': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a new feature that led to catching the bug. |
||
// todo test this case | ||
log.fatal(projectType.error.message) | ||
break | ||
default: | ||
casesHandled(projectType) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -533,6 +533,7 @@ async function scaffoldBaseFiles(options: InternalConfig) { | |
fs.writeAsync(Path.join(options.projectRoot, 'package.json'), { | ||
name: options.projectName, | ||
license: 'UNLICENSED', | ||
version: '0.0.0', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixing the bug |
||
dependencies: {}, | ||
scripts: { | ||
format: "npx prettier --write './**/*.{ts,md}'", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,7 +133,6 @@ describe('tsconfig', () => { | |
}, | ||
], | ||
"rootDir": ".", | ||
"skipLibCheck": true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is reverting a temp feature we had in the release series. |
||
"strict": true, | ||
"target": "es2016", | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import { rightOrThrow } from '@nexus/logger/dist/utils' | ||
import Chalk from 'chalk' | ||
import { stripIndent } from 'common-tags' | ||
import { Either, isLeft, left, right } from 'fp-ts/lib/Either' | ||
|
@@ -6,9 +7,10 @@ import * as Path from 'path' | |
import * as ts from 'ts-morph' | ||
import { PackageJson } from 'type-fest' | ||
import type { ParsedCommandLine } from 'typescript' | ||
import { findFile, findFileRecurisvelyUpwardSync } from '../../lib/fs' | ||
import { findFile, isEmptyDir } from '../../lib/fs' | ||
import { START_MODULE_NAME } from '../../runtime/start/start-module' | ||
import { rootLogger } from '../nexus-logger' | ||
import * as PJ from '../package-json' | ||
import * as PackageManager from '../package-manager' | ||
import { createContextualError } from '../utils' | ||
import { readOrScaffoldTsconfig } from './tsconfig' | ||
|
@@ -194,7 +196,10 @@ export async function create(options?: Options): Promise<Either<Error, Layout>> | |
|
||
// TODO lodash merge defaults or something | ||
|
||
const scanResult = await scan({ cwd, entrypointPath: normalizedEntrypoint }) | ||
const errScanResult = await scan({ cwd, entrypointPath: normalizedEntrypoint }) | ||
if (isLeft(errScanResult)) return errScanResult | ||
const scanResult = errScanResult.right | ||
|
||
const buildInfo = getBuildInfo(options?.buildOutputDir, scanResult, options?.asBundle) | ||
|
||
log.trace('layout build info', { data: buildInfo }) | ||
|
@@ -255,17 +260,24 @@ export function createFromData(layoutData: Data): Layout { | |
* Analyze the user's project files/folders for how conventions are being used | ||
* and where key modules exist. | ||
*/ | ||
export async function scan(opts?: { cwd?: string; entrypointPath?: string }): Promise<ScanResult> { | ||
export async function scan(opts?: { | ||
cwd?: string | ||
entrypointPath?: string | ||
}): Promise<Either<Error, ScanResult>> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scan is now an either, since it cares about failed reads of package.json now. |
||
log.trace('starting scan') | ||
const projectRoot = opts?.cwd ?? process.cwd() | ||
const packageManagerType = await PackageManager.detectProjectPackageManager({ projectRoot }) | ||
const maybePackageJson = findPackageJson({ projectRoot }) | ||
const maybeErrPackageJson = PJ.findRecurisvelyUpwardSync({ cwd: projectRoot }) | ||
const maybeAppModule = opts?.entrypointPath ?? findAppModule({ projectRoot }) | ||
const tsConfig = await readOrScaffoldTsconfig({ | ||
projectRoot, | ||
}) | ||
const nexusModules = findNexusModules(tsConfig, maybeAppModule) | ||
|
||
if (maybeErrPackageJson && isLeft(maybeErrPackageJson.contents)) { | ||
return maybeErrPackageJson.contents | ||
} | ||
|
||
const result: ScanResult = { | ||
app: | ||
maybeAppModule === null | ||
|
@@ -277,7 +289,9 @@ export async function scan(opts?: { cwd?: string; entrypointPath?: string }): Pr | |
project: readProjectInfo(opts), | ||
tsConfig, | ||
packageManagerType, | ||
packageJson: maybePackageJson, | ||
packageJson: maybeErrPackageJson | ||
? { ...maybeErrPackageJson, content: rightOrThrow(maybeErrPackageJson.contents) } | ||
: maybeErrPackageJson, | ||
} | ||
|
||
log.trace('completed scan', { result }) | ||
|
@@ -288,7 +302,7 @@ export async function scan(opts?: { cwd?: string; entrypointPath?: string }): Pr | |
process.exit(1) | ||
} | ||
|
||
return result | ||
return right(result) | ||
} | ||
|
||
// todo allow user to configure these for their project | ||
|
@@ -347,46 +361,45 @@ export async function scanProjectType(opts: { | |
cwd: string | ||
}): Promise< | ||
| { type: 'unknown' | 'new' } | ||
| { type: 'malformed_package_json'; error: PJ.MalformedPackageJsonError } | ||
| { | ||
type: 'NEXUS_project' | 'node_project' | ||
packageJson: {} | ||
packageJsonLocation: { path: string; dir: string } | ||
} | ||
> { | ||
const packageJsonLocation = findPackageJsonRecursivelyUpward(opts) | ||
const packageJson = PJ.findRecurisvelyUpwardSync(opts) | ||
|
||
if (packageJsonLocation === null) { | ||
if (await isEmptyCWD()) { | ||
if (packageJson === null) { | ||
if (await isEmptyDir(opts.cwd)) { | ||
return { type: 'new' } | ||
} | ||
return { type: 'unknown' } | ||
} | ||
|
||
const packageJson = FS.read(packageJsonLocation.path, 'json') | ||
if (packageJson?.dependencies?.['nexus']) { | ||
if (isLeft(packageJson.contents)) { | ||
return { | ||
type: 'malformed_package_json', | ||
error: packageJson.contents.left, | ||
} | ||
} | ||
|
||
const pjc = rightOrThrow(packageJson.contents) // will never throw, check above | ||
if (pjc.dependencies?.['nexus']) { | ||
return { | ||
type: 'NEXUS_project', | ||
packageJson: packageJsonLocation, | ||
packageJsonLocation: packageJsonLocation, | ||
packageJson: packageJson, | ||
packageJsonLocation: packageJson, | ||
} | ||
} | ||
|
||
return { | ||
type: 'node_project', | ||
packageJson: packageJsonLocation, | ||
packageJsonLocation: packageJsonLocation, | ||
packageJson: packageJson, | ||
packageJsonLocation: packageJson, | ||
} | ||
} | ||
|
||
/** | ||
* Check if the CWD is empty of any files or folders. | ||
* TODO we should make nice exceptions for known meaningless files, like .DS_Store | ||
*/ | ||
async function isEmptyCWD(): Promise<boolean> { | ||
const contents = await FS.listAsync() | ||
return contents === undefined || contents.length === 0 | ||
} | ||
|
||
const ENV_VAR_DATA_NAME = 'NEXUS_LAYOUT' | ||
|
||
export function saveDataForChildProcess(layout: Layout): { NEXUS_LAYOUT: string } { | ||
|
@@ -433,33 +446,6 @@ function readProjectInfo(opts?: { cwd?: string }): ScanResult['project'] { | |
} | ||
} | ||
|
||
/** | ||
* Find the package.json file path. Looks recursively upward to disk root. | ||
* Starts looking in CWD If no package.json found along search, returns null. | ||
*/ | ||
function findPackageJsonRecursivelyUpward(opts: { cwd: string }) { | ||
return findFileRecurisvelyUpwardSync('package.json', opts) | ||
} | ||
|
||
/** | ||
* | ||
*/ | ||
function findPackageJson(opts: { projectRoot: string }): ScanResult['packageJson'] { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have a new lib module for dealing with package json |
||
const packageJsonPath = FS.path(opts.projectRoot, 'package.json') | ||
|
||
try { | ||
const content = FS.read(packageJsonPath, 'json') | ||
|
||
return { | ||
content, | ||
path: packageJsonPath, | ||
dir: Path.dirname(packageJsonPath), | ||
} | ||
} catch { | ||
return null | ||
} | ||
} | ||
|
||
function normalizeEntrypoint(entrypoint: string | undefined, cwd: string): Either<Error, string | undefined> { | ||
if (!entrypoint) { | ||
return right(undefined) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
import { Either, isLeft, left, right, toError, tryCatch } from 'fp-ts/lib/Either' | ||
import * as FS from 'fs-jetpack' | ||
import { isEmpty, isPlainObject, isString } from 'lodash' | ||
import parseJson from 'parse-json' | ||
import * as Path from 'path' | ||
import { PackageJson } from 'type-fest' | ||
import { execeptionType } from './utils' | ||
|
||
const malformedPackageJson = execeptionType<'MalformedPackageJson', { path: string; reason: string }>( | ||
'MalformedPackageJson', | ||
(c) => `package.json at ${c.path} was malformed\n\n${c.reason}` | ||
) | ||
|
||
export type MalformedPackageJsonError = ReturnType<typeof malformedPackageJson> | ||
|
||
export type Result = { | ||
path: string | ||
dir: string | ||
contents: Either<MalformedPackageJsonError, PackageJson> | ||
} | null | ||
|
||
/** | ||
* Find the package.json file path. Looks recursively upward to disk root. | ||
* Starts looking in CWD If no package.json found along search, returns null. | ||
* If packge.json fonud but fails to be parsed or fails validation than an error is returned. | ||
*/ | ||
export function findRecurisvelyUpwardSync(opts: { cwd: string }): Result { | ||
let found: Result = null | ||
let currentDir = opts.cwd | ||
const localFS = FS.cwd(currentDir) | ||
|
||
while (true) { | ||
const filePath = Path.join(currentDir, 'package.json') | ||
const rawContents = localFS.read(filePath) | ||
|
||
if (rawContents) { | ||
const contents = parse(rawContents, filePath) | ||
found = { dir: currentDir, path: filePath, contents } | ||
break | ||
} | ||
|
||
if (currentDir === '/') { | ||
break | ||
} | ||
|
||
currentDir = Path.join(currentDir, '..') | ||
} | ||
|
||
return found | ||
} | ||
|
||
/** | ||
* Parse package.json contents. | ||
*/ | ||
export function parse(contents: string, path: string) { | ||
const errRawData = tryCatch( | ||
() => parseJson(contents, path), | ||
(e) => malformedPackageJson({ path, reason: toError(e).message }) | ||
) | ||
if (isLeft(errRawData)) return errRawData | ||
const rawData = errRawData.right | ||
console.log(rawData) | ||
if (!isPlainObject(rawData)) | ||
return left(malformedPackageJson({ path, reason: 'Package.json data is not an object' })) | ||
if (!isString(rawData.name)) | ||
return left(malformedPackageJson({ path, reason: 'Package.json name field is not a string' })) | ||
if (isEmpty(rawData.name)) | ||
return left(malformedPackageJson({ path, reason: 'Package.json name field is empty' })) | ||
if (!isString(rawData.version)) | ||
return left(malformedPackageJson({ path, reason: 'Package.json version field is not a string' })) | ||
if (isEmpty(rawData.version)) | ||
return left(malformedPackageJson({ path, reason: 'Package.json version field is empty' })) | ||
return right(rawData as PackageJson & { name: string; version: string }) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -326,6 +326,23 @@ export type ContextualError<Context extends Record<string, unknown> = {}> = Erro | |
context: Context | ||
} | ||
|
||
export interface Exception<T extends string, C extends SomeRecord> extends Error { | ||
type: T | ||
context: C | ||
} | ||
|
||
export function execeptionType<Type extends string, Context extends SomeRecord>( | ||
jasonkuhrt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type: Type, | ||
template: (ctx: Context) => string | ||
) { | ||
return (ctx: Context) => { | ||
const e = new Error(template(ctx)) as Exception<Type, Context> | ||
e.type = type | ||
e.context = ctx | ||
return e | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not satisfied with the usability but it seems to get the job done which is a two step processes where we:
I am open to using classes but didn't see how that would help. I hope we can make this better in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find it interesting as a first iteration and certainly not bad. Let's simply see how that works in practice for now and iterate if it turns out to not work well! |
||
|
||
/** | ||
* Create an error with contextual data about it. | ||
* | ||
|
@@ -335,7 +352,7 @@ export type ContextualError<Context extends Record<string, unknown> = {}> = Erro | |
* strongly typed with the Either contstruct, making it so the error contextual | ||
* data flows with inference through your program. | ||
*/ | ||
export function createContextualError<Context extends Record<string, unknown>>( | ||
export function createContextualError<Context extends SomeRecord>( | ||
message: string, | ||
context: Context | ||
): ContextualError<Context> { | ||
|
@@ -417,3 +434,5 @@ export function prettyImportPath(id: string): string { | |
|
||
return id | ||
} | ||
|
||
type SomeRecord = Record<string, unknown> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to improve dx around the bad feedback given by JSON.parse