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

feat: update to current best practices #394

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
6 changes: 2 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,16 +185,14 @@ export async function main() {
const dependencies = [];
const devDependencies = [
"@octokit/tsconfig",
"@vitest/coverage-v8",
"esbuild",
"glob",
"@types/jest",
"@types/node",
"jest",
"prettier",
"semantic-release",
"semantic-release-plugin-update-version-in-files",
"ts-jest",
"typescript",
"vitest",
];

if (answers.isPlugin || answers.isAuthenticationStrategy) {
Expand Down
63 changes: 41 additions & 22 deletions lib/create-esbuild-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ export { createEsbuildScript };
import { writePrettyFile } from "./write-pretty-file.js";

async function createEsbuildScript(answers) {
const nodeBuildOptions = ` // Build a CJS Node.js bundle
const nodeBuildOptions = ` // Build an ESM Node.js bundle
Copy link
Member

Choose a reason for hiding this comment

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

I feel we are at a point where we should test the output of running this script is the expected one. Do you think it's worth investing in this?

await esbuild.build({
entryPoints,
outdir: "pkg/dist-node",
bundle: true,
platform: "node",
target: "node18",
format: "cjs",
format: "esm",
...sharedOptions,
})`;
const browserBuildOptions = ` // Build an ESM browser bundle
Expand All @@ -22,27 +22,39 @@ async function createEsbuildScript(answers) {
format: "esm",
...sharedOptions,
});`;
const dualBuildOptions = ` await Promise.all([
// Build a CJS Node.js bundle
esbuild.build({
const dualBuildOptions = ` await esbuild.build({
entryPoints,
outdir: "pkg/dist-node",
outdir: "pkg/dist-bundle",
bundle: true,
platform: "node",
target: "node18",
format: "cjs",
...sharedOptions,
}),
// Build an ESM browser bundle
esbuild.build({
entryPoints,
outdir: "pkg/dist-web",
bundle: true,
platform: "browser",
platform: "neutral",
format: "esm",
...sharedOptions,
}),
]);`;
const nodeJSExports = `exports: {
".": {
types: "./dist-types/index.d.ts",
oscard0m marked this conversation as resolved.
Show resolved Hide resolved
import: "./dist-node/index.js",
// Tooling currently are having issues with the "exports" field when there is no "default", ex: TypeScript, eslint
default: "./dist-node/index.js",
},
}`;
const browserExports = `exports: {
".": {
types: "./dist-types/index.d.ts",
oscard0m marked this conversation as resolved.
Show resolved Hide resolved
import: "./dist-browser/index.js",
// Tooling currently are having issues with the "exports" field when there is no "default", ex: TypeScript, eslint
default: "./dist-browser/index.js",
},
}`;
const dualExports = `exports: {
".": {
types: "./dist-types/index.d.ts",
oscard0m marked this conversation as resolved.
Show resolved Hide resolved
import: "./dist-bundle/index.js",
// Tooling currently are having issues with the "exports" field when there is no "default", ex: TypeScript, eslint
oscard0m marked this conversation as resolved.
Show resolved Hide resolved
default: "./dist-bundle/index.js",
},
}`;

await writePrettyFile(
"scripts/esbuild.mjs",
Expand Down Expand Up @@ -88,11 +100,11 @@ async function main() {
const entryPoints = ["./pkg/dist-src/index.js"];\n
${
answers.supportsBrowsers && answers.supportsNode
? dualBuildOptions
? dualExports
: answers.supportsNode
? nodeBuildOptions
? nodeJSExports
: answers.supportsBrowsers
? browserBuildOptions
? browserExports
: ""
}\n
// Copy the README, LICENSE to the pkg folder
Expand All @@ -112,10 +124,17 @@ async function main() {
{
...pkg,
files: ["dist-*/**", "bin/**"],
main: "dist-node/index.js",
module: "dist-web/index.js",
types: "dist-types/index.d.ts",
source: "dist-src/index.js",
${
answers.supportsBrowsers && answers.supportsNode
? dualBuildOptions
: answers.supportsNode
? nodeBuildOptions
: answers.supportsBrowsers
? browserBuildOptions
: ""
},\n
sideEffects: false,
},
null,
Expand Down
24 changes: 1 addition & 23 deletions lib/create-package-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function createPackageJson(answers) {
lint: "prettier --check '{src,test}/**/*' README.md package.json",
"lint:fix": "prettier --write '{src,test}/**/*' README.md package.json",
pretest: "npm run -s lint",
test: "jest --coverage",
test: "vitest run --coverage",
},
repository: `github:${answers.repository}`,
keywords: ["github", "api", "sdk", "toolkit"],
Expand All @@ -25,29 +25,7 @@ async function createPackageJson(answers) {
dependencies: {},
devDependencies: {},
peerDependencies: {},
jest: {
transform: {
"^.+\\.tsx?$": [
"ts-jest",
{
tsconfig: "test/tsconfig.json",
},
],
},
moduleNameMapper: {
"^(.+)\\.jsx?$": "$1",
},
},
release: {
branches: [
"+([0-9]).x",
"main",
"next",
{
name: "beta",
prerelease: true,
},
],
oscard0m marked this conversation as resolved.
Show resolved Hide resolved
plugins: [
"@semantic-release/commit-analyzer",
"@semantic-release/release-notes-generator",
Expand Down
25 changes: 25 additions & 0 deletions lib/create-vitest-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { writePrettyFile } from "./write-pretty-file.js";
export { createVitestConfig };

async function createVitestConfig() {
const config = `import { defineConfig } from "vite";

export default defineConfig({
test: {
coverage: {
include: ["src/**/*.ts"],
exclude: ["src/methods/get-oauth-client-code.ts"], // Exclude this file from coverage as it isn't exported
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope

reporter: ["html"],
thresholds: {
branches: 98,
functions: 100,
lines: 100,
statements: 100,
},
oscard0m marked this conversation as resolved.
Show resolved Hide resolved
},
},
});
`;

await writePrettyFile("vitest.config.js", config);
}
Loading