Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
10 changes: 10 additions & 0 deletions apps/nxls/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this needed for something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without it, i get

> nx run nxls:package

asset yarn.lock 53.1 KiB [emitted]
asset README.md 9.69 KiB [emitted] [from: ../../README.md] [copied]
asset main.js 4.07 KiB [emitted] [minimized] (name: main)
asset package.json 1.43 KiB [emitted]
asset bin/nxls 44 bytes [emitted] [from: src/bin/nxls] [copied]
asset 3rdpartylicenses.txt 1 bytes [emitted]


ERROR in ./src/main.ts
Module build failed (from ../../node_modules/@nx/webpack/src/utils/web-babel-loader.js):
Error: Cannot find module '/home/forivall/code/public/nx-console/apps/nxls/.babelrc'
Require stack:
- /home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/config/files/configuration.js
- /home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/config/files/index.js
- /home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/index.js
- /home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/babel-loader/lib/index.js
- /home/forivall/code/public/nx-console/node_modules/@nx/webpack/src/utils/web-babel-loader.js
- /home/forivall/code/public/nx-console/node_modules/loader-runner/lib/loadLoader.js
- /home/forivall/code/public/nx-console/node_modules/loader-runner/lib/LoaderRunner.js
- /home/forivall/code/public/nx-console/node_modules/webpack/lib/NormalModuleFactory.js
- /home/forivall/code/public/nx-console/node_modules/webpack/lib/Compiler.js
- /home/forivall/code/public/nx-console/node_modules/webpack/lib/webpack.js
- /home/forivall/code/public/nx-console/node_modules/webpack/lib/index.js
- /home/forivall/code/public/nx-console/node_modules/@nx/webpack/src/executors/webpack/lib/run-webpack.js
- /home/forivall/code/public/nx-console/node_modules/@nx/webpack/src/executors/webpack/webpack.impl.js
- /home/forivall/code/public/nx-console/node_modules/nx/src/config/schema-utils.js
- /home/forivall/code/public/nx-console/node_modules/nx/src/command-line/run/executor-utils.js
- /home/forivall/code/public/nx-console/node_modules/nx/src/project-graph/utils/project-configuration-utils.js
- /home/forivall/code/public/nx-console/node_modules/nx/src/utils/package-json.js
- /home/forivall/code/public/nx-console/node_modules/nx/src/utils/print-help.js
- /home/forivall/code/public/nx-console/node_modules/nx/src/command-line/run/run.js
- /home/forivall/code/public/nx-console/node_modules/nx/bin/run-executor.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1420:15)
    at resolve (node:internal/modules/helpers:163:19)
    at loadConfig (/home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/config/files/configuration.js:213:5)
    at loadConfig.next (<anonymous>)
    at buildRootChain (/home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/config/config-chain.js:64:47)
    at buildRootChain.next (<anonymous>)
    at loadPrivatePartialConfig (/home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/config/partial.js:72:62)
    at loadPrivatePartialConfig.next (<anonymous>)
    at loadPartialConfig (/home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/config/partial.js:115:25)
    at loadPartialConfig.next (<anonymous>)
    at step (/home/forivall/code/public/nx-console/node_modules/gensync/index.js:261:32)
    at evaluateAsync (/home/forivall/code/public/nx-console/node_modules/gensync/index.js:291:5)
    at /home/forivall/code/public/nx-console/node_modules/gensync/index.js:93:9
    at new Promise (<anonymous>)
    at async (/home/forivall/code/public/nx-console/node_modules/gensync/index.js:92:14)
    at stopHiding - secret - don't use this - v1 (/home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/errors/rewrite-stack-trace.js:47:12)
    at Object.loadPartialConfigAsync (/home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/@babel/core/lib/config/index.js:34:85)
    at Object.loader (/home/forivall/code/public/nx-console/node_modules/@nx/webpack/node_modules/babel-loader/lib/index.js:116:30)

webpack 5.101.3 compiled with 1 error (60dc9cbac55a2a52)

"presets": [
[
"@nx/js/babel",
{
"useBuiltIns": "usage"
}
]
]
}
11 changes: 6 additions & 5 deletions apps/nxls/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ import { URI } from 'vscode-uri';
import { ensureOnlyJsonRpcStdout } from './ensureOnlyJsonRpcStdout';
import { NativeWatcher } from '@nx-console/shared-watcher';

const connection = createConnection(ProposedFeatures.all);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a convenience - it shows a better error message when you run nxls without any arguments (as the error message says "use --stdio or one of the other modes"

Copy link
Collaborator

Choose a reason for hiding this comment

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

that makes sense, an error during connection creation is one of the few ones we don't want to suppress

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, it throws an error, then the unhandled exception while creating the error tries to use the connection, which is not initialized, and so before this change, you would just get an unhelpful ReferenceError: Cannot access 'connection' before initialization. After this change, it reports Error: Connection input stream is not set. Use arguments of createConnection or set command line parameters: '--node-ipc', '--stdio' or '--socket={number}'


process.on('unhandledRejection', (e: any) => {
connection.console.error(formatError(`Unhandled exception`, e));
});
Expand All @@ -124,8 +126,6 @@ let unregisterFileWatcher: () => Promise<void> = async () => {
};
let reconfigureAttempts = 0;

const connection = createConnection(ProposedFeatures.all);

// Create a text document manager.
const documents = new TextDocuments(TextDocument);

Expand All @@ -138,12 +138,13 @@ connection.onInitialize(async (params) => {
lspLogger.log('Initializing Nx Language Server');

const { workspacePath } = params.initializationOptions ?? {};
const extractFsPath = (p: string | undefined) => p && URI.parse(p).fsPath;
try {
WORKING_PATH =
workspacePath ||
params.workspaceFolders?.[0]?.uri ||
extractFsPath(workspacePath) ||
extractFsPath(params.workspaceFolders?.[0]?.uri) ||
params.rootPath ||
URI.parse(params.rootUri ?? '').fsPath;
extractFsPath(params.rootUri ?? '');
Comment on lines +141 to +147
Copy link
Contributor Author

Choose a reason for hiding this comment

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

helix always passes a uri for the workspace path.


if (!WORKING_PATH) {
throw 'Unable to determine workspace path';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ export async function getProjectGraphOutput(workspacePath: string) {
async function getCacheDir(workspacePath: string): Promise<string> {
const importPath = await findNxPackagePath(
workspacePath,
join('src', 'utils', 'cache-directory.js')
join('src', 'utils', 'cache-directory.js'),
);

if (!importPath) {
lspLogger.log(
`Unable to load the "nx" package from the workspace. Please ensure that the proper dependencies are installed locally.`
`Unable to load the "nx" package from the workspace. Please ensure that the proper dependencies are installed locally.`,
);
throw 'local Nx dependency not found';
}
Expand Down
3 changes: 2 additions & 1 deletion libs/shared/file-system/src/lib/directory-exists.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { URI } from 'vscode-uri';
import { stat } from 'fs/promises';

export async function directoryExists(filePath: string): Promise<boolean> {
try {
return (await stat(filePath)).isDirectory();
return (await stat(URI.parse(filePath).fsPath)).isDirectory();
} catch {
return false;
}
Expand Down
6 changes: 5 additions & 1 deletion libs/shared/npm/src/lib/workspace-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { gte, NxVersion } from '@nx-console/nx-version';
import { directoryExists } from '@nx-console/shared-file-system';
import type { Logger } from '@nx-console/shared-utils';
import { stat } from 'fs/promises';
import Module from 'module';
import type { ProjectGraphProjectNode } from 'nx/src/devkit-exports';
import { platform } from 'os';
import { join } from 'path';
Expand Down Expand Up @@ -80,13 +81,16 @@ export async function workspaceDependencyPath(
export function importWorkspaceDependency<T>(
importPath: string,
logger?: Logger,
workspacePath: string = __dirname,
): Promise<T> {
if (platform() === 'win32') {
importPath = importPath.replace(/\\/g, '/');
}

const workspaceRequire = Module.createRequire(workspacePath);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const imported = require(importPath);
const imported = workspaceRequire(importPath);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the webpack bundler in nx run nxls:package will replace this require to a webpack require that always errors. using module and createRequire is one of a few solutions. (imo, this would take a larger refactor to make use of the workspacePath argument, but it could simplify a lot of logic)

Copy link
Collaborator

Choose a reason for hiding this comment

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

are you saying that this always errors? or only for helix?
This is a widely used function so I'm hesitant to change it and I wonder why this is causing issues in helix but not otherwise...

Copy link
Contributor Author

@forivall forivall Sep 27, 2025

Choose a reason for hiding this comment

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

this will probably error with other editors when using nxls (such as neovim). Note that this is using the output from nx run nxls:package, which uses webpack, while nxls:build uses esbuild. I tried changing a few things with webpack to get it to work, but I couldnt figure it out (we should all know webpack woes, as it does a lot of "clever" things). The vscode builder uses vsce from the esbuild output, so that's why it doesnt hit this issue.

Basically, the packaging for nxls is kinda broken right now, and fixing this in a different way would need changing how things are packaged/bundled. My priority was to just get it to work 😢. (i think this is the solution)

Copy link
Contributor Author

@forivall forivall Sep 27, 2025

Choose a reason for hiding this comment

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

Yeah, i've changed the build in the latest commit(s), and it should work as it was originally designed.

While comparing the compiled output, i also noticed that loading the yarn pnp api was broken in the same way, so i also included that in the regex for modules to use the __non_webpack_require__ replacement. Alternatively, you could switch bundlers away from webpack, but I'm not a maintainer, so i'm not going to make that change as my initial proposal.

Copy link
Collaborator

Choose a reason for hiding this comment

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

okay thanks for the investigation @forivall
Tbh I'd rather switch away from webpack to esbuild and align the builds between build/package than start patching up the webpack config and adding a new dependency and everything.

Is this something you'd be willing to do in this PR? Or should I try the migration and then you can build on top of that?

Copy link
Contributor Author

@forivall forivall Sep 29, 2025

Choose a reason for hiding this comment

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

yeah, i can do it. the build target already does most of what package does, so i think it would just be a noop with a dependency on build, and fix the assets and other configurations in the build output to get the missing bin script to copy, etc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

perfect, I'll be glad to get rid of webpack here - I already have done some work to enable properly publishing the nxls to npm again so when the package command is fixed, we can do that as well which should help


logger?.log(`Using local Nx package at ${importPath}`);

Expand Down
Loading