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

Follow symlinks in resolveRequest #16369

Closed
Closed
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"npm-run-all": "4.1.5",
"nprogress": "0.2.0",
"pixrem": "5.0.0",
"pnpm": "5.8.0",
"postcss-nested": "4.2.1",
"postcss-pseudoelements": "5.0.0",
"postcss-short-size": "4.0.0",
Expand Down
47 changes: 22 additions & 25 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import { fileExists } from '../lib/file-exists'
import { getPackageVersion } from '../lib/get-package-version'
import { Rewrite } from '../lib/load-custom-routes'
import { resolveRequest } from '../lib/resolve-request'
import { ResolvedModule, resolveRequest } from '../lib/resolve-request'
import { getTypeScriptConfiguration } from '../lib/typescript/getTypeScriptConfiguration'
import {
CLIENT_STATIC_FILES_RUNTIME_MAIN,
Expand Down Expand Up @@ -308,7 +308,7 @@ export default async function getBaseWebpackConfig(

let typeScriptPath: string | undefined
try {
typeScriptPath = resolveRequest('typescript', `${dir}/`)
typeScriptPath = resolveRequest('typescript', `${dir}/`).resolvedPath
} catch (_) {}
const tsConfigPath = path.join(dir, 'tsconfig.json')
const useTypeScript = Boolean(
Expand Down Expand Up @@ -617,7 +617,7 @@ export default async function getBaseWebpackConfig(
// Resolve the import with the webpack provided context, this
// ensures we're resolving the correct version when multiple
// exist.
let res: string
let res: ResolvedModule
try {
res = resolveRequest(request, `${context}/`)
} catch (err) {
Expand All @@ -629,21 +629,17 @@ export default async function getBaseWebpackConfig(

// Same as above, if the request cannot be resolved we need to have
// webpack "bundle" it so it surfaces the not found error.
if (!res) {
if (!res.resolvedPath) {
return callback()
}

let isNextExternal: boolean = false
if (isLocal) {
// we need to process next-server/lib/router/router so that
// the DefinePlugin can inject process.env values
isNextExternal = /next[/\\]dist[/\\]next-server[/\\](?!lib[/\\]router[/\\]router)/.test(
res
)

if (!isNextExternal) {
return callback()
}
// we need to process next-server/lib/router/router so that
// the DefinePlugin can inject process.env values
const isNextExternal = /next[/\\]dist[/\\]next-server[/\\](?!lib[/\\]router[/\\]router)/.test(
res.originalPath
)
if (isLocal && !isNextExternal) {
return callback()
}

// `isNextExternal` special cases Next.js' internal requires that
Expand All @@ -655,7 +651,7 @@ export default async function getBaseWebpackConfig(
// This means we need to make sure its request resolves to the same
// package that'll be available at runtime. If it's not identical,
// we need to bundle the code (even if it _should_ be external).
let baseRes: string | null
let baseRes: ResolvedModule | null
try {
baseRes = resolveRequest(request, `${dir}/`)
} catch (err) {
Expand All @@ -665,32 +661,32 @@ export default async function getBaseWebpackConfig(
// Same as above: if the package, when required from the root,
// would be different from what the real resolution would use, we
// cannot externalize it.
if (baseRes !== res) {
if (!baseRes || baseRes.resolvedPath !== res.resolvedPath) {
return callback()
}
}

// Default pages have to be transpiled
if (
!res.match(/next[/\\]dist[/\\]next-server[/\\]/) &&
(res.match(/[/\\]next[/\\]dist[/\\]/) ||
!res.originalPath.match(/next[/\\]dist[/\\]next-server[/\\]/) &&
(res.originalPath.match(/[/\\]next[/\\]dist[/\\]/) ||
// This is the @babel/plugin-transform-runtime "helpers: true" option
res.match(/node_modules[/\\]@babel[/\\]runtime[/\\]/))
res.originalPath.match(/node_modules[/\\]@babel[/\\]runtime[/\\]/))
) {
return callback()
}

// Webpack itself has to be compiled because it doesn't always use module relative paths
if (
res.match(/node_modules[/\\]webpack/) ||
res.match(/node_modules[/\\]css-loader/)
res.originalPath.match(/node_modules[/\\]webpack/) ||
res.originalPath.match(/node_modules[/\\]css-loader/)
) {
return callback()
}

// Anything else that is standard JavaScript within `node_modules`
// can be externalized.
if (isNextExternal || res.match(/node_modules[/\\].*\.js$/)) {
if (isNextExternal || res.originalPath.match(/node_modules[/\\].*\.js$/)) {
const externalRequest = isNextExternal
? // Generate Next.js external import
path.posix.join(
Expand All @@ -700,7 +696,7 @@ export default async function getBaseWebpackConfig(
.relative(
// Root of Next.js package:
path.join(__dirname, '..'),
res
res.resolvedPath
)
// Windows path normalization
.replace(/\\/g, '/')
Expand Down Expand Up @@ -1444,14 +1440,15 @@ export default async function getBaseWebpackConfig(
? '@zeit/next-stylus'
: 'next'
)
)
).resolvedPath

// If we found `@zeit/next-css` ...
if (correctNextCss) {
// ... resolve the version of `css-loader` shipped with that
// package instead of whichever was hoisted highest in your
// `node_modules` tree.
const correctCssLoader = resolveRequest(use.loader, correctNextCss)
.resolvedPath
if (correctCssLoader) {
// We saved the user from a failed build!
use.loader = correctCssLoader
Expand Down
2 changes: 1 addition & 1 deletion packages/next/build/webpack/config/blocks/css/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ async function loadPlugin(
throw new Error(genericErrorText)
}

const pluginPath = resolveRequest(pluginName, `${dir}/`)
const pluginPath = resolveRequest(pluginName, `${dir}/`).resolvedPath
if (isIgnoredPlugin(pluginPath)) {
return false
} else if (options === true) {
Expand Down
27 changes: 15 additions & 12 deletions packages/next/compiled/resolve/LICENSE
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
This software is released under the MIT license:
MIT License

Permission is hereby granted, free of charge, to any person obtaining a copy of
this software and associated documentation files (the "Software"), to deal in
the Software without restriction, including without limitation the rights to
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
the Software, and to permit persons to whom the Software is furnished to do so,
subject to the following conditions:
Copyright (c) 2012 James Halliday

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
2 changes: 1 addition & 1 deletion packages/next/compiled/resolve/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/next/lib/get-package-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export async function getPackageVersion({
: `${cwd}/`

try {
const targetPath = resolveRequest(`${name}/package.json`, cwd2)
const targetPath = resolveRequest(`${name}/package.json`, cwd2).resolvedPath
const targetContent = await fs.readFile(targetPath, 'utf-8')
return JSON5.parse(targetContent).version ?? null
} catch {
Expand Down
41 changes: 38 additions & 3 deletions packages/next/lib/resolve-request.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,53 @@
import resolve from 'next/dist/compiled/resolve/index.js'
import path from 'path'

export function resolveRequest(req: string, issuer: string): string {
export interface ResolvedModule {
/**
* The first path on disk which the module resolves to, equivalent to running Node.js with
* `--preserve-symlinks`.
*/
originalPath: string
/**
* The resolved module on disk, after following symbolic links. This emulates Node.js's default
* module resolution behavior.
*/
resolvedPath: string
}

export function resolveRequest(req: string, issuer: string): ResolvedModule {
// The `resolve` package is prebuilt through ncc, which prevents
// PnP from being able to inject itself into it. To circumvent
// this, we simply use PnP directly when available.
if (process.versions.pnp) {
const { resolveRequest: pnpResolveRequest } = require(`pnpapi`)
return pnpResolveRequest(req, issuer, { considerBuiltins: false })
const modulePath = pnpResolveRequest(req, issuer, {
considerBuiltins: false,
})
return {
originalPath: modulePath,
resolvedPath: modulePath,
}
} else {
const basedir =
issuer.endsWith(path.posix.sep) || issuer.endsWith(path.win32.sep)
? issuer
: path.dirname(issuer)
return resolve.sync(req, { basedir })

const originalPath = resolve.sync(req, {
basedir,
preserveSymlinks: true,
})
const resolvedPath = resolve.sync(req, {
basedir,
// Node.js's built-in module resolution resolves symbolic links by default,
// however the `resolve@1` package does not. By passing `preserveSymlinks: false`, we override
// resolve@1's default of `true` so that Node.js's default behaviour is emulated.
preserveSymlinks: false,
})

return {
originalPath,
resolvedPath,
}
}
}
5 changes: 4 additions & 1 deletion packages/next/lib/typescript/hasNecessaryDependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ export async function hasNecessaryDependencies(

const missingPackages = requiredPackages.filter((p) => {
try {
resolutions.set(p.pkg, resolveRequest(p.file, path.join(baseDir, '/')))
resolutions.set(
p.pkg,
resolveRequest(p.file, path.join(baseDir, '/')).resolvedPath
)
return false
} catch (_) {
return true
Expand Down
4 changes: 2 additions & 2 deletions packages/next/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@
"@types/react": "16.9.17",
"@types/react-dom": "16.9.4",
"@types/react-is": "16.7.1",
"@types/resolve": "0.0.8",
"@types/resolve": "1.17.1",
"@types/semver": "7.3.1",
"@types/send": "0.14.4",
"@types/styled-jsx": "2.2.8",
Expand Down Expand Up @@ -199,7 +199,7 @@
"postcss-preset-env": "6.7.0",
"raw-body": "2.4.1",
"recast": "0.18.5",
"resolve": "1.11.0",
"resolve": "1.17.0",
"semver": "7.3.2",
"send": "0.17.1",
"source-map": "0.6.1",
Expand Down
14 changes: 14 additions & 0 deletions test/integration/pnpm/app/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "pnpm-app",
"version": "1.0.0",
"private": true,
"scripts": {
"dev": "next dev",
"build": "next build",
"start": "next start"
},
"dependencies": {
"react": "^16.7.0",
"react-dom": "^16.7.0"
}
}
1 change: 1 addition & 0 deletions test/integration/pnpm/app/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => <h1>Hello World</h1>
89 changes: 89 additions & 0 deletions test/integration/pnpm/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/* eslint-env jest */
import execa from 'execa'
import fs from 'fs-extra'
import os from 'os'
import path from 'path'

const pnpmExecutable = path.join(
__dirname,
'..',
'..',
'..',
'..',
'node_modules',
'.bin',
'pnpm'
)
const nextPackageDir = path.join(
__dirname,
'..',
'..',
'..',
'..',
'packages',
'next'
)
const appDir = path.join(__dirname, '..', 'app')

jest.setTimeout(1000 * 60 * 5)

const runNpm = (cwd, ...args) => execa('npm', [...args], { cwd })
const runPnpm = (cwd, ...args) => execa(pnpmExecutable, [...args], { cwd })

async function usingTempDir(fn) {
const folder = path.join(os.tmpdir(), Math.random().toString(36).substring(2))
await fs.mkdirp(folder)
try {
return await fn(folder)
} finally {
await fs.remove(folder)
}
}

async function packNextTarball(cwd) {
const { stdout } = await runNpm(
cwd,
'pack',
'--ignore-scripts', // Skip the prepublish script
nextPackageDir
)
const tarballFilename = stdout.match(/.*\.tgz/)[0]

if (!tarballFilename) {
throw new Error(
`pnpm failed to pack "next" package tarball in directory ${nextPackageDir}.`
)
}

return path.join(cwd, tarballFilename)
}

describe('pnpm support', () => {
it('should build with dependencies installed via pnpm', async () => {
// Create a Next.js app in a temporary directory, and install dependencies with pnpm.
// "next" is installed by packing a tarball from 'next.js/packages/next', because installing
// "next" directly via `pnpm add path/to/next.js/packages/next` results in a symlink:
// 'app/node_modules/next' -> 'path/to/next.js/packages/next'. This is undesired since modules
// inside "next" would be resolved to the next.js monorepo 'node_modules'; installing from a
// tarball avoids this issue.
await usingTempDir(async (tempDir) => {
const nextTarballPath = await packNextTarball(tempDir)

const tempAppDir = path.join(tempDir, 'app')
await fs.copy(appDir, tempAppDir)
await runPnpm(tempAppDir, 'install')
await runPnpm(tempAppDir, 'add', nextTarballPath)

expect(
await fs.pathExists(path.join(tempAppDir, 'pnpm-lock.yaml'))
).toBeTruthy()
expect(
require(path.join(tempAppDir, 'package.json')).dependencies['next']
).toMatch(/^file:/)

const { stdout, stderr } = await runPnpm(tempAppDir, 'run', 'build')
console.log(stdout, stderr)
expect(stdout).toMatch(/Compiled successfully/)
})
})
})
Loading