Skip to content

Commit

Permalink
fix: detect extraneous files
Browse files Browse the repository at this point in the history
  • Loading branch information
Skn0tt committed Oct 23, 2023
1 parent a0a27f0 commit e206df4
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 20 deletions.
35 changes: 17 additions & 18 deletions node/npm_dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,23 @@ const getNPMSpecifiers = async (
const npmSpecifiersWithExtraneousFiles = new Set<string>()

for (const [packageJsonPath, reason] of reasons.entries()) {
const parents = [...reason.parents]
const isExtraneousFile = reason.type.every((type) => type === 'asset')

// An extraneous file is a dependency that was traced by NFT and marked
// as not being statically imported. We can't process dynamic importing
// at runtime, so we gather the list of modules that may use these files
// so that we can warn users about this caveat.
if (isExtraneousFile) {
parents.forEach((path) => {
const specifier = getPackageName(path)

if (specifier) {
npmSpecifiersWithExtraneousFiles.add(specifier)
}
})
}

// every dependency will have its `package.json` in `reasons` exactly once.
// by only looking at this file, we save us from doing duplicate work.
const isPackageJson = packageJsonPath.endsWith('package.json')
Expand All @@ -132,8 +149,6 @@ const getNPMSpecifiers = async (
const packageName = getPackageName(packageJsonPath)
if (packageName === undefined) continue

const parents = [...reason.parents]

const isDirectDependency = parents.some((parentPath) => {
if (!parentPath.startsWith(`node_modules${path.sep}`)) return true
// typically, edge functions have no direct dependency on the `package.json` of a module.
Expand All @@ -152,22 +167,6 @@ const getNPMSpecifiers = async (
types: referenceTypes ? await safelyDetectTypes(path.join(basePath, packageJsonPath)) : undefined,
})
}

const isExtraneousFile = reason.type.every((type) => type === 'asset')

// An extraneous file is a dependency that was traced by NFT and marked
// as not being statically imported. We can't process dynamic importing
// at runtime, so we gather the list of modules that may use these files
// so that we can warn users about this caveat.
if (isExtraneousFile) {
parents.forEach((path) => {
const specifier = getPackageName(path)

if (specifier) {
npmSpecifiersWithExtraneousFiles.add(specifier)
}
})
}
}

return {
Expand Down
5 changes: 3 additions & 2 deletions node/server/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ test('Starts a server and serves requests for edge functions', async () => {
getFunctionsConfig: true,
}

const { features, functionsConfig, graph, success } = await server(
const { features, functionsConfig, graph, success, npmSpecifiersWithExtraneousFiles } = await server(
functions,
{
very_secret_secret: 'i love netlify',
Expand All @@ -57,6 +57,7 @@ test('Starts a server and serves requests for edge functions', async () => {
expect(features).toEqual({ npmModules: true })
expect(success).toBe(true)
expect(functionsConfig).toEqual([{ path: '/my-function' }, {}, { path: '/global-netlify' }])
expect(npmSpecifiersWithExtraneousFiles).toEqual(['dictionary'])

for (const key in functions) {
const graphEntry = graph?.modules.some(
Expand Down Expand Up @@ -104,7 +105,7 @@ test('Starts a server and serves requests for edge functions', async () => {
`/// <reference types="${join('..', '..', '..', 'node_modules', 'id', 'types.d.ts')}" />`,
)

const identidadeBarrelFile = await readFile(join(servePath, 'barrel-1.js'), 'utf-8')
const identidadeBarrelFile = await readFile(join(servePath, 'barrel-2.js'), 'utf-8')
expect(identidadeBarrelFile).toContain(
`/// <reference types="${join(
'..',
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/serve_test/netlify/edge-functions/echo_env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ import { yell } from 'helper'
import id from 'id'
import identidade from '@pt-committee/identidade'

import { getWords } from 'dictionary'

// this will throw since FS access is not available in Edge Functions.
// but we need this line so that `dictionary` is scanned for extraneous dependencies
try {
getWords()
} catch {}

export default () => {
return new Response(yell(identidade(id(Deno.env.get('very_secret_secret'))) ?? ''))
}
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/serve_test/node_modules/dictionary/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions test/fixtures/serve_test/node_modules/dictionary/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions test/fixtures/serve_test/node_modules/dictionary/words.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit e206df4

Please sign in to comment.