-
Notifications
You must be signed in to change notification settings - Fork 5
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
cleanup #45
cleanup #45
Changes from 9 commits
9b86106
6d88840
a5cc76f
e95f6b7
21c75a1
962db91
e7b5b33
8e4323c
91d3d33
3660a05
4ac42dc
207e3d9
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 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,7 +1,7 @@ | ||||||||||||||
import { execa } from "execa"; | ||||||||||||||
import { Extension, isDefined, Options, TemplateDescriptor } from "../types"; | ||||||||||||||
import { Options, TemplateDescriptor } from "../types"; | ||||||||||||||
import { baseDir } from "../utils/consts"; | ||||||||||||||
import { extensionDict } from "../utils/extensions-tree"; | ||||||||||||||
import { extensionDict } from "../utils/extensions-dictionary"; | ||||||||||||||
import { findFilesRecursiveSync } from "../utils/find-files-recursively"; | ||||||||||||||
import { mergePackageJson } from "../utils/merge-package-json"; | ||||||||||||||
import fs from "fs"; | ||||||||||||||
|
@@ -16,64 +16,21 @@ const EXTERNAL_EXTENSION_TMP_FOLDER = "tmp-external-extension"; | |||||||||||||
const copy = promisify(ncp); | ||||||||||||||
let copyOrLink = copy; | ||||||||||||||
|
||||||||||||||
const expandExtensions = (options: Options): Extension[] => { | ||||||||||||||
const expandedExtensions = options.extensions | ||||||||||||||
.map(extension => extensionDict[extension]) | ||||||||||||||
.map(extDescriptor => [extDescriptor.extends, extDescriptor.value].filter(isDefined)) | ||||||||||||||
.flat() | ||||||||||||||
// this reduce just removes duplications | ||||||||||||||
.reduce((exts, ext) => (exts.includes(ext) ? exts : [...exts, ext]), [] as Extension[]); | ||||||||||||||
|
||||||||||||||
return expandedExtensions; | ||||||||||||||
}; | ||||||||||||||
technophile-04 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
|
||||||||||||||
const isTemplateRegex = /([^/\\]*?)\.template\./; | ||||||||||||||
const isPackageJsonRegex = /package\.json/; | ||||||||||||||
const isYarnLockRegex = /yarn\.lock/; | ||||||||||||||
const isNextGeneratedRegex = /packages\/nextjs\/generated/; | ||||||||||||||
const isConfigRegex = /([^/\\]*?)\\config\.json/; | ||||||||||||||
const isArgsRegex = /([^/\\]*?)\.args\./; | ||||||||||||||
const isExtensionFolderRegex = /extensions$/; | ||||||||||||||
const isPackagesFolderRegex = /packages$/; | ||||||||||||||
|
||||||||||||||
const copyBaseFiles = async ({ dev: isDev }: Options, basePath: string, targetDir: string) => { | ||||||||||||||
const copyBaseFiles = async (basePath: string, targetDir: string) => { | ||||||||||||||
await copyOrLink(basePath, targetDir, { | ||||||||||||||
clobber: false, | ||||||||||||||
filter: fileName => { | ||||||||||||||
// NOTE: filter IN | ||||||||||||||
const isTemplate = isTemplateRegex.test(fileName); | ||||||||||||||
const isPackageJson = isPackageJsonRegex.test(fileName); | ||||||||||||||
const isYarnLock = isYarnLockRegex.test(fileName); | ||||||||||||||
const isNextGenerated = isNextGeneratedRegex.test(fileName); | ||||||||||||||
|
||||||||||||||
const skipAlways = isTemplate || isPackageJson; | ||||||||||||||
const skipDevOnly = isYarnLock || isNextGenerated; | ||||||||||||||
const shouldSkip = skipAlways || (isDev && skipDevOnly); | ||||||||||||||
|
||||||||||||||
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. Trying to understand this.
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. Ohh my bad actually I planned to write a comment on why did I remove all this and forgot about it but here is description: So basically this function copies "base" dir into user mentioned path.
We were ignore package.json here because just below this create-eth/src/tasks/copy-template-files.ts Lines 57 to 62 in 6a3afc7
I have removed this part in this PR since:
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 initially thought it was not needed since we have removed "nextjs/generated" but just updated the logic and added it back so that we don't symlink |
||||||||||||||
return !shouldSkip; | ||||||||||||||
return !isTemplate; | ||||||||||||||
}, | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
const basePackageJsonPaths = findFilesRecursiveSync(basePath, path => isPackageJsonRegex.test(path)); | ||||||||||||||
|
||||||||||||||
basePackageJsonPaths.forEach(packageJsonPath => { | ||||||||||||||
const partialPath = packageJsonPath.split(basePath)[1]; | ||||||||||||||
mergePackageJson(path.join(targetDir, partialPath), path.join(basePath, partialPath), isDev); | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
if (isDev) { | ||||||||||||||
const baseYarnLockPaths = findFilesRecursiveSync(basePath, path => isYarnLockRegex.test(path)); | ||||||||||||||
baseYarnLockPaths.forEach(yarnLockPath => { | ||||||||||||||
const partialPath = yarnLockPath.split(basePath)[1]; | ||||||||||||||
void copy(path.join(basePath, partialPath), path.join(targetDir, partialPath)); | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
const nextGeneratedPaths = findFilesRecursiveSync(basePath, path => isNextGeneratedRegex.test(path)); | ||||||||||||||
nextGeneratedPaths.forEach(nextGeneratedPath => { | ||||||||||||||
const partialPath = nextGeneratedPath.split(basePath)[1]; | ||||||||||||||
void copy(path.join(basePath, partialPath), path.join(targetDir, partialPath)); | ||||||||||||||
}); | ||||||||||||||
} | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
const copyExtensionsFiles = async ({ dev: isDev }: Options, extensionPath: string, targetDir: string) => { | ||||||||||||||
|
@@ -211,22 +168,22 @@ const processTemplatedFiles = async ( | |||||||||||||
); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// ToDo. Bug, if arg not present in arg[0], but present in arg[1], it will not be added. | ||||||||||||||
const allKeys = [...new Set(args.flatMap(Object.keys))]; | ||||||||||||||
|
||||||||||||||
const freshArgs: { [key: string]: string[] } = Object.fromEntries( | ||||||||||||||
Object.keys(args[0] ?? {}).map(key => [ | ||||||||||||||
allKeys.map(key => [ | ||||||||||||||
key, // INFO: key for the freshArgs object | ||||||||||||||
[], // INFO: initial value for the freshArgs object | ||||||||||||||
]), | ||||||||||||||
); | ||||||||||||||
|
||||||||||||||
const combinedArgs = args.reduce<typeof freshArgs>((accumulated, arg) => { | ||||||||||||||
Object.entries(arg).map(([key, value]) => { | ||||||||||||||
accumulated[key]?.push(value); | ||||||||||||||
}); | ||||||||||||||
return accumulated; | ||||||||||||||
}, freshArgs); | ||||||||||||||
|
||||||||||||||
// TODO test: if first arg file found only uses 1 name, I think the rest are not used? | ||||||||||||||
|
||||||||||||||
const output = fileTemplate(combinedArgs); | ||||||||||||||
|
||||||||||||||
const targetPath = path.join( | ||||||||||||||
|
@@ -290,34 +247,31 @@ export async function copyTemplateFiles(options: Options, templateDir: string, t | |||||||||||||
const tmpDir = path.join(targetDir, EXTERNAL_EXTENSION_TMP_FOLDER); | ||||||||||||||
|
||||||||||||||
// 1. Copy base template to target directory | ||||||||||||||
await copyBaseFiles(options, basePath, targetDir); | ||||||||||||||
|
||||||||||||||
// 2. Add "parent" extensions (set via config.json#extend field) | ||||||||||||||
options.extensions = expandExtensions(options); | ||||||||||||||
Comment on lines
-295
to
-296
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 no more need this since we are not allowing extending extension anymore |
||||||||||||||
await copyBaseFiles(basePath, targetDir); | ||||||||||||||
|
||||||||||||||
// 3. Copy extensions folders | ||||||||||||||
// 2. Copy extensions folders | ||||||||||||||
await Promise.all( | ||||||||||||||
options.extensions.map(async extension => { | ||||||||||||||
const extensionPath = extensionDict[extension].path; | ||||||||||||||
await copyExtensionsFiles(options, extensionPath, targetDir); | ||||||||||||||
}), | ||||||||||||||
); | ||||||||||||||
|
||||||||||||||
// 4. Set up external extension if needed | ||||||||||||||
// 3. Set up external extension if needed | ||||||||||||||
if (options.externalExtension) { | ||||||||||||||
await setUpExternalExtensionFiles(options, tmpDir); | ||||||||||||||
await copyExtensionsFiles(options, path.join(tmpDir, "extension"), targetDir); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// 5. Process templated files and generate output | ||||||||||||||
// 4. Process templated files and generate output | ||||||||||||||
await processTemplatedFiles(options, basePath, targetDir); | ||||||||||||||
|
||||||||||||||
// 6. Delete tmp directory | ||||||||||||||
// 5. Delete tmp directory | ||||||||||||||
if (options.externalExtension) { | ||||||||||||||
await fs.promises.rm(tmpDir, { recursive: true }); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// 7. Initialize git repo to avoid husky error | ||||||||||||||
// 6. Initialize git repo to avoid husky error | ||||||||||||||
await execa("git", ["init"], { cwd: targetDir }); | ||||||||||||||
await execa("git", ["checkout", "-b", "main"], { cwd: targetDir }); | ||||||||||||||
} |
technophile-04 marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
import { fileURLToPath } from "url"; | ||
import path from "path"; | ||
import fs from "fs"; | ||
import { Extension, ExtensionDescriptor, ExtensionDict } from "../types"; | ||
|
||
// global object to store mapping of extension name and its descriptor | ||
export const extensionDict: ExtensionDict = {} as ExtensionDict; | ||
|
||
const currentFileUrl = import.meta.url; | ||
|
||
const templatesDirectory = path.resolve(decodeURI(fileURLToPath(currentFileUrl)), "../../templates"); | ||
|
||
/** | ||
* This function has side effects. It initializes the extensionDict. | ||
*/ | ||
const initializeExtensionsDict = (basePath: string) => { | ||
const extensionsPath = path.resolve(basePath, "extensions"); | ||
let extensions: Extension[]; | ||
try { | ||
extensions = fs.readdirSync(extensionsPath) as Extension[]; | ||
} catch (error) { | ||
console.error(`Couldn't read the extensions directory: ${extensionsPath}`); | ||
return; | ||
} | ||
|
||
extensions.forEach(ext => { | ||
const extPath = path.resolve(extensionsPath, ext); | ||
const configPath = path.resolve(extPath, "config.json"); | ||
|
||
let config: Record<string, string> = {}; | ||
try { | ||
config = JSON.parse(fs.readFileSync(configPath, "utf8")) as Record<string, string>; | ||
} catch (error) { | ||
if (fs.existsSync(configPath)) { | ||
throw new Error( | ||
`Couldn't parse existing config.json file. | ||
Extension: ${ext}; | ||
Config file path: ${configPath}`, | ||
); | ||
} | ||
} | ||
const name = config.name ?? ext; | ||
const value = ext; | ||
|
||
const extDescriptor: ExtensionDescriptor = { | ||
name, | ||
value, | ||
path: extPath, | ||
}; | ||
|
||
extensionDict[ext] = extDescriptor; | ||
}); | ||
}; | ||
|
||
// This function call will run only once due to Node.js module caching in first import of file | ||
// it won't run again even if imported in multiple files. | ||
initializeExtensionsDict(templatesDirectory); |
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. Haven't removed this file it's just rename checkout #45 (comment) |
This file was deleted.
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.
What if we change this filename to
core-extensions
or something like that? So then we can createexternal-extensions
for extensions that will live in other repos. Or are we planning to put all of them together here?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.
voting for first option