Skip to content

Commit

Permalink
feat(import): detect node built-ins (#6609)
Browse files Browse the repository at this point in the history
This PR adds the ability to detect Node built-ins that are being
imported.

**Notes:**
- We currently detect all imports in project files, without checking if
the importing file is in the import chain of the app itself. This _may_
result in false positives, if one of the files in the project that does
not belong to the app imports a Node module.
- For that reason - currently this only triggers a "warning" and not a
critical error.
- We compare the built-ins that we find with the project dependencies,
in case some of them are shimmed.
- We're using a static list taken from here:
https://github.com/sindresorhus/builtin-modules. We can't use the
package directly since it is an ESM module, and our tests cannot import
it, and in any case all it does is exporting this JSON.

If you clone this branch you can test it using
http://localhost:8000/p?accessLevel=public&clone=liady/utopia-vite-project
- I added an `import * as fs from 'fs'` there to test this
functionality. (don't forget to turn on the `Import Wizard` feature
flag)

<img width="658" alt="image"
src="https://github.com/user-attachments/assets/c4ddedef-d863-45fa-89f6-63c7ee6cd4f4">

**Manual Tests:**
I hereby swear that:

- [X] I opened a hydrogen project and it loaded
- [X] I could navigate to various routes in Play mode
  • Loading branch information
liady authored Nov 1, 2024
1 parent 71e3817 commit 07c6bd8
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
[
"assert",
"assert/strict",
"async_hooks",
"buffer",
"child_process",
"cluster",
"console",
"constants",
"crypto",
"dgram",
"diagnostics_channel",
"dns",
"dns/promises",
"domain",
"events",
"fs",
"fs/promises",
"http",
"http2",
"https",
"inspector",
"inspector/promises",
"module",
"net",
"os",
"path",
"path/posix",
"path/win32",
"perf_hooks",
"process",
"punycode",
"querystring",
"readline",
"readline/promises",
"repl",
"stream",
"stream/consumers",
"stream/promises",
"stream/web",
"string_decoder",
"timers",
"timers/promises",
"tls",
"trace_events",
"tty",
"url",
"util",
"util/types",
"v8",
"vm",
"wasi",
"worker_threads",
"zlib"
]
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ import {
type RequirementCheck,
type RequirementCheckResult,
} from '../utopia-requirements-types'
import { getProjectDependencies } from '../../../../../components/assets'
import { getProjectDependencies, walkContentsTree } from '../../../../../components/assets'
import { isParseSuccess, isTextFile } from '../../../../../core/shared/project-file-types'
import builtinModules from './builtin-modules.json'

const serverPackagesRestrictionList: RegExp[] = [/^next/, /^remix/, /^astro/, /^svelte/]

export default class CheckServerPackages implements RequirementCheck {
check(projectContents: ProjectContentTreeRoot): RequirementCheckResult {
const projectDependencies = getProjectDependencies(projectContents) ?? {}

// check for server packages in dependencies
const serverPackages = Object.keys(projectDependencies).filter((packageName) =>
serverPackagesRestrictionList.some((restriction) => restriction.test(packageName)),
)
Expand All @@ -21,9 +25,43 @@ export default class CheckServerPackages implements RequirementCheck {
resultValue: serverPackages.join(', '),
}
}

// check for node builtins in imports
const nodeBuiltins: string[] = []
walkContentsTree(projectContents, (fullPath, file) => {
if (isTextFile(file)) {
const parseResult = file.fileContents.parsed
if (isParseSuccess(parseResult)) {
for (const importSource of Object.keys(parseResult.imports)) {
// if it's a node builtin and not shimmed as a dependency, add it to the list
if (isBuiltinModule(importSource) && projectDependencies[importSource] == null) {
nodeBuiltins.push(importSource)
}
}
}
}
})
if (nodeBuiltins.length > 0) {
return {
resolution: RequirementResolutionResult.Partial,
resultText: 'Node built-ins found',
resultValue: nodeBuiltins.join(', '),
}
}
return {
resolution: RequirementResolutionResult.Passed,
resultText: 'No server packages found',
}
}
}

const moduleSet = new Set(builtinModules)
const NODE_PROTOCOL = 'node:'
function isBuiltinModule(moduleName: string) {
let moduleNameWithoutNodeProtocol = moduleName
if (moduleName.startsWith(NODE_PROTOCOL)) {
moduleNameWithoutNodeProtocol = moduleName.slice(NODE_PROTOCOL.length)
}

return moduleSet.has(moduleNameWithoutNodeProtocol)
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { DefaultPackageJson } from '../../../../../components/editor/store/edito
import { getPackageJson } from '../../../../../components/assets'
import CheckStoryboard from './requirement-storyboard'
import CheckServerPackages from './requirement-server-packages'
import { parseProjectContents } from '../../../../../sample-projects/sample-project-utils.test-utils'

describe('requirements checks', () => {
describe('project language', () => {
Expand Down Expand Up @@ -219,5 +220,59 @@ describe('requirements checks', () => {
const result = check.check(projectContents)
expect(result.resolution).toBe(RequirementResolutionResult.Critical)
})

it('should return partial for a project with node builtins', () => {
const check = new CheckServerPackages()
const project = simpleDefaultProject({
additionalFiles: {
'/src/app.js': textFile(
textFileContents(
`import { readFileSync } from 'fs'
import * as crypto from 'node:crypto'`,
unparsed,
RevisionsState.CodeAhead,
),
null,
null,
0,
),
},
})
const parsedProjectContents = parseProjectContents(project.projectContents)
const result = check.check(parsedProjectContents)
expect(result.resolution).toBe(RequirementResolutionResult.Partial)
expect(result.resultValue).toBe('fs, node:crypto')
})

it('should return success for a project with node builtins that are shimmed', () => {
const check = new CheckServerPackages()
const project = simpleDefaultProject({
additionalFiles: {
'/src/app.js': textFile(
textFileContents(
`import { readFileSync } from 'fs'`,
unparsed,
RevisionsState.CodeAhead,
),
null,
null,
0,
),
'/package.json': textFile(
textFileContents(
JSON.stringify({ dependencies: { fs: '1.0.0' } }, null, 2),
unparsed,
RevisionsState.CodeAhead,
),
null,
null,
0,
),
},
})
const parsedProjectContents = parseProjectContents(project.projectContents)
const result = check.check(parsedProjectContents)
expect(result.resolution).toBe(RequirementResolutionResult.Passed)
})
})
})

0 comments on commit 07c6bd8

Please sign in to comment.