Skip to content

Commit

Permalink
fix: fix JSON imports for ES modules (#982)
Browse files Browse the repository at this point in the history
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
ehmicky and kodiakhq[bot] committed Feb 8, 2022
1 parent 225f387 commit 64089d8
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .eslintrc.cjs
Expand Up @@ -2,6 +2,11 @@ const { overrides } = require('@netlify/eslint-config-node')

module.exports = {
extends: '@netlify/eslint-config-node',
rules: {
// This rule enforces using Buffers with `JSON.parse()`. However, TypeScript
// does not recognize yet that `JSON.parse()` accepts Buffers as argument.
'unicorn/prefer-json-parse-buffer': 'off',
},
overrides: [
...overrides,
{
Expand Down
4 changes: 2 additions & 2 deletions src/runtimes/node/bundlers/zisi/traverse.ts
@@ -1,3 +1,4 @@
import { promises as fs } from 'fs'
import { dirname } from 'path'

import { nonNullable } from '../../../../utils/non_nullable'
Expand Down Expand Up @@ -74,8 +75,7 @@ const getDependenciesForModuleName = async function ({
state.modulePaths.add(modulePath)

// The path depends on the user's build, i.e. must be dynamic
// eslint-disable-next-line import/no-dynamic-require, node/global-require, @typescript-eslint/no-var-requires
const packageJson = require(packagePath)
const packageJson = JSON.parse(await fs.readFile(packagePath, 'utf8'))

const [publishedFiles, sideFiles, depsPaths] = await Promise.all([
getPublishedFiles(modulePath),
Expand Down
5 changes: 3 additions & 2 deletions src/runtimes/node/utils/package_json.ts
@@ -1,3 +1,5 @@
import { promises as fs } from 'fs'

import pkgDir from 'pkg-dir'

interface PackageJson {
Expand Down Expand Up @@ -38,8 +40,7 @@ const getPackageJson = async function (srcDir: string): Promise<PackageJson> {
const packageJsonPath = `${packageRoot}/package.json`
try {
// The path depends on the user's build, i.e. must be dynamic
// eslint-disable-next-line import/no-dynamic-require, node/global-require, @typescript-eslint/no-var-requires
const packageJson = require(packageJsonPath)
const packageJson = JSON.parse(await fs.readFile(packageJsonPath, 'utf8'))
return sanitisePackageJson(packageJson)
} catch (error) {
throw new Error(`${packageJsonPath} is invalid JSON: ${error.message}`)
Expand Down
7 changes: 4 additions & 3 deletions tests/bin.js
@@ -1,18 +1,19 @@
const { promises: fs } = require('fs')
const { join } = require('path')

const test = require('ava')
const execa = require('execa')
const { tmpName } = require('tmp-promise')

const { version } = require('../package.json')

const { FIXTURES_DIR, BINARY_PATH } = require('./helpers/main')

const ROOT_PACKAGE_JSON = `${__dirname}/../package.json`

const exec = (args, options) => execa('node', [BINARY_PATH, ...args], options)

test('CLI | --version', async (t) => {
const { stdout } = await exec(['--version'])

const { version } = JSON.parse(await fs.readFile(ROOT_PACKAGE_JSON))
t.is(stdout, version)
})

Expand Down
6 changes: 2 additions & 4 deletions tests/main.js
Expand Up @@ -2461,8 +2461,7 @@ test('Creates a manifest file with the list of created functions if the `manifes
},
})

// eslint-disable-next-line import/no-dynamic-require, node/global-require
const manifest = require(manifestPath)
const manifest = JSON.parse(await readFile(manifestPath))

t.is(manifest.version, 1)
t.is(manifest.system.arch, arch)
Expand Down Expand Up @@ -2584,8 +2583,7 @@ testMany(

files.every((file) => t.is(file.schedule, schedule))

// eslint-disable-next-line import/no-dynamic-require, node/global-require
const manifest = require(manifestPath)
const manifest = JSON.parse(await readFile(manifestPath))

manifest.functions.forEach((fn) => {
t.is(fn.schedule, schedule)
Expand Down

1 comment on commit 64089d8

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

⏱ Benchmark results

largeDepsEsbuild: 7.7s

largeDepsNft: 32.8s

largeDepsZisi: 1m 0.4s

Please sign in to comment.