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

chore(build): validate if exporting is correct in package.json and jsr.json #3638

Merged
merged 8 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
15 changes: 12 additions & 3 deletions build/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,30 @@

/// <reference types="bun-types/bun" />

import fs, { write } from 'fs'
import path from 'path'
import arg from 'arg'
import { $, stdout } from 'bun'
import { build } from 'esbuild'
import type { Plugin, PluginBuild, BuildOptions } from 'esbuild'
import * as glob from 'glob'
import fs from 'fs'
import path from 'path'
import { removePrivateFields } from './remove-private-fields'
import { $, stdout } from 'bun'
import { validateExports } from './validate-exports'

const args = arg({
'--watch': Boolean,
})

const isWatch = args['--watch'] || false

const readJsonExports = (path: string) => JSON.parse(fs.readFileSync(path, 'utf-8')).exports

const [packageJsonExports, jsrJsonExports] = ['./package.json', './jsr.json'].map(readJsonExports)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it also should validate typesVersions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, will work on it.

Copy link
Member

Choose a reason for hiding this comment

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

@nakasyou Ah, you are right. But perhaps validating typeVersions is another issue? We can check whether it's correct without jsr.json.

@EdamAme-x What do you think of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to write the code, but you are right, it may be another problem.  
I will consider this after this PR is merged.

Copy link
Member

Choose a reason for hiding this comment

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

Okay. Let's merge this first.


// Validate exports of package.json and jsr.json
validateExports(packageJsonExports, jsrJsonExports, 'jsr.json')
validateExports(jsrJsonExports, packageJsonExports, 'package.json')

const entryPoints = glob.sync('./src/**/*.ts', {
ignore: ['./src/**/*.test.ts', './src/mod.ts', './src/middleware.ts', './src/deno/**/*.ts'],
})
Expand Down
4 changes: 2 additions & 2 deletions build/remove-private-fields.test.ts
EdamAme-x marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/// <reference types="vitest/globals" />

import { removePrivateFields } from './remove-private-fields'
import fs from 'node:fs/promises'
import path from 'node:path'
import os from 'node:os'
import path from 'node:path'
import { removePrivateFields } from './remove-private-fields'

describe('removePrivateFields', () => {
it('Works', async () => {
Expand Down
31 changes: 31 additions & 0 deletions build/validate-exports.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/// <reference types="vitest/globals" />

import { validateExports } from './validate-exports'

const mockExports1 = {
'./a': './a.ts',
'./b': './b.ts',
'./c/a': './c.ts',
'./d/*': './d/*.ts',
}

const mockExports2 = {
'./a': './a.ts',
'./b': './b.ts',
'./c/a': './c.ts',
'./d/a': './d/a.ts',
}

const mockExports3 = {
'./a': './a.ts',
'./c/a': './c.ts',
'./d/*': './d/*.ts',
}

describe('validateExports', () => {
it('Works', async () => {
expect(() => validateExports(mockExports1, mockExports1, 'package.json')).not.toThrowError()
expect(() => validateExports(mockExports1, mockExports2, 'jsr.json')).not.toThrowError()
expect(() => validateExports(mockExports1, mockExports3, 'package.json')).toThrowError()
})
})
37 changes: 37 additions & 0 deletions build/validate-exports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
export const validateExports = (
source: Record<string, unknown>,
target: Record<string, unknown>,
fileName: string
) => {
const isEntryInTarget = (entry: string): boolean => {
if (entry in target) {
return true
}

// e.g., "./utils/*" -> "./utils"
const wildcardPrefix = entry.replace(/\/\*$/, '')
if (entry.endsWith('/*')) {
return Object.keys(target).some(
(targetEntry) =>
targetEntry.startsWith(wildcardPrefix + '/') && targetEntry !== wildcardPrefix
)
}

const separatedEntry = entry.split('/')
while (separatedEntry.length > 0) {
const pattern = `${separatedEntry.join('/')}/*`
if (pattern in target) {
return true
}

Check warning on line 25 in build/validate-exports.ts

View check run for this annotation

Codecov / codecov/patch

build/validate-exports.ts#L24-L25

Added lines #L24 - L25 were not covered by tests
separatedEntry.pop()
}

return false
}

Object.keys(source).forEach((sourceEntry) => {
if (!isEntryInTarget(sourceEntry)) {
throw new Error(`Missing "${sourceEntry}" in '${fileName}'`)
}
})
}
1 change: 1 addition & 0 deletions jsr.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"./testing": "./src/helper/testing/index.ts",
"./dev": "./src/helper/dev/index.ts",
"./ws": "./src/helper/websocket/index.ts",
"./conninfo": "./src/helper/conninfo/index.ts",
"./utils/body": "./src/utils/body.ts",
"./utils/buffer": "./src/utils/buffer.ts",
"./utils/color": "./src/utils/color.ts",
Expand Down
8 changes: 8 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
"import": "./dist/index.js",
"require": "./dist/cjs/index.js"
},
"./request": {
"types": "./dist/types/request.d.ts",
"import": "./dist/request.js",
"require": "./dist/cjs/request.js"
},
"./types": {
"types": "./dist/types/types.d.ts",
"import": "./dist/types.js",
Expand Down Expand Up @@ -387,6 +392,9 @@
},
"typesVersions": {
"*": {
"request": [
"./dist/types/request"
],
"types": [
"./dist/types/types"
],
Expand Down
4 changes: 2 additions & 2 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default defineConfig({
},
test: {
globals: true,
include: ['**/src/**/(*.)+(spec|test).+(ts|tsx|js)', '**/scripts/**/(*.)+(spec|test).+(ts|tsx|js)'],
include: ['**/src/**/(*.)+(spec|test).+(ts|tsx|js)', '**/scripts/**/(*.)+(spec|test).+(ts|tsx|js)', '**/build/**/(*.)+(spec|test).+(ts|tsx|js)'],
exclude: [...configDefaults.exclude, '**/sandbox/**', '**/*.case.test.+(ts|tsx|js)'],
setupFiles: ['./.vitest.config/setup-vitest.ts'],
coverage: {
Expand All @@ -20,7 +20,7 @@ export default defineConfig({
...(configDefaults.coverage.exclude ?? []),
'benchmarks',
'runtime-tests',
'build.ts',
'build/build.ts',
'src/test-utils',
'perf-measures',

Expand Down
Loading