[scss] link doesn't work with ~Tilde to search in node module or webpack alias

[Friksel]

Using a **~**Tilde to go directly to a node_module path without having to add aliases to webpack this path doesn't get resolved by VS Code.

So with a node_module 'bootstrap' installed, we can import bootstrap sass-files using this construction:

@import '~bootstrap/scss/bootstrap-grid';
@import '~bootstrap/scss/utilities/sizing';

While this is working fine with Webpack and the sass-loader/node_sass, VS Code doesn't recognise this tilde-construction and searches on a follow-link in our source-folder\bootstrap\ instead of in node_modules\bootstrap\

When we use aliases in Javascript we solved his resolve problem by adding aliases to the jsconfig.json. But AFAIK there's not such a config file for Sass in VS Code.
I've been searching everywhere, including settings on sass, but couldn't find anything where we could make VS Code respect the node_modules path (and Webpack aliases) with a tilde~.

So unfortunately we seem to not being able to click to follow link on these sass imports.

versions

• VSCode Version: 1.37.0
• OS Version: Windows 10 Home Updated

Steps to Reproduce:

1. Have a node modulle, like node_modules/bootstrap, installed
2. Import a sassfile from this module, like @import '~bootstrap/scss/bootstrap-grid';
3. Ctrl+click on the import line to try to 'Follow Link'
   Result: it shows an error-message popup saying 'Unable to open' with the wrong path

extensions
Does this issue occur when all extensions are disabled?: Yes

[Friksel]

Some extra information:

I see there happened to be a pull request here on exactly this and it seems to be merged, but for some reason doesn't work?:
#70693

Documentation webpack about imports with a tilde:
https://webpack.js.org/loaders/sass-loader/#imports

[penx]

I had this working but was unaware that the distributed version of vscode has parts of the code webpacked, including where I had written the resolve function, so webpack complained and the code was reverted.

This was rewritten by @aeschli but the rewritten version doesn't work in most use cases as far as I'm aware.

We need to either find a way to get this to work after being processed by webpack, or have the code sit in a non-webpacked part of vscode (if that's exists/is possible) and have the code called from there.

Unfortunately my knowledge of why and how vscode is using webpack for the distributable is beyond me and I have no idea where to search for help on this.

[penx]

Perhaps someone from the vscode team can answer:

• why is vscode using webpack for the production build? Is this so it can be used within a browser?
• is the entirety of vscode being passed through webpack?
• how does vscode use node module resolution when using 'go to module definition' inside a js file?
• If not, is it possible to write helper functions in part of the vscode that is not being webpacked, then use them from within the code that is?

For some context to the last point, see extensions/css-language-features/server/src/utils/documentContext.ts:resolvePathToModule

vscode/extensions/css-language-features/server/src/utils/documentContext.ts

Lines 22 to 30 in 36ae347

	function resolvePathToModule(_moduleName: string, _relativeTo: string): string | undefined {
	// resolve the module relative to the document. We can't use `require` here as the code is webpacked.
	const documentFolder = dirname(URI.parse(_relativeTo).fsPath);
	const packPath = join(documentFolder, 'node_modules', _moduleName, 'package.json');
	if (existsSync(packPath)) {
	return URI.file(packPath).toString();
	}
	return undefined;
	}

I want resolvePathToModule to use node module resolution if vscode is running within a node environment. I can't use require.resolve directly as webpack will complain, so either I need to call out to a helper function that can or somehow recreate what require.resolve is doing manually (this would probably need file system access though so would likely have the same issue).

[aeschli]

webpack is probably the best packager out there. For browsers and also for commonjs. It's a pity that its so restrictive with require as it I could really allow require.resolve(module, {path: [...]})

is it possible to write helper functions in part of the vscode that is not being webpacked

In the webpack.config.js you set a list of node modules that should not be bundled but just required.

Alternative is to reimplement the require lookup. I think that's the typescript language server does:
https://github.com/microsoft/TypeScript/blob/01e1b1bb276f48eaa0612a34dd8f1eec5d86bd49/src/compiler/moduleNameResolver.ts

[penx]

I just remembered this

https://github.com/microsoft/vscode-languageserver-node/blob/4006192ed8e5d1cd1ffd888461cf830c3898b21b/server/src/resolve.ts#L17-L24

Which I think is what's calling require.resolve when you do jump to definition on a .js file.

I assume there's a way we can use require.resolve in the same way? Will look in to it a bit more

[penx]

webpack is probably the best packager out there

I'm not questioning the choice of webpack specifically, just why there's a need to package at all and not just run from modular, transpiled code

[penx]

https://github.com/browserify/resolve may be an option

[penx]

In the webpack.config.js you set a list of node modules that should not be bundled but just required.

@aeschli I think you mentioned this before but I couldn't find it. Do you mean in this file?

https://github.com/microsoft/vscode/blob/master/extensions/shared.webpack.config.js

Or this one?

https://github.com/microsoft/vscode/blob/master/extensions/css-language-features/server/extension.webpack.config.js

Or another?

[aeschli]

It would go into extension.webpack.config.js.
Example:

externals: {
			'vscode': 'commonjs vscode', // ignored because it doesn't exist
		},

[penx]

I've drafted something here #79651 but not had a chance to test yet, will hopefully pick up later or tomorrow

[octref]

I propose that we don't add one-liner dependencies to just bypass webpack.

Does doing it this way work for your cases?

• Find filepath
• Find workspace folder path
• For each folder along the way, determine if node_modules/<module-name> exists
• If so resolve into it

For example, if you have /foo/bar/baz.js and your workspace folder is /foo, and you are requiring ~bootstrap/something.css, it would try:

• /foo/bar/node_modules/boostrap/something.css
• /foo/node_modules/boostrap/something.css
• Stop here, even though require would try /node_modules/boostrap/something.css, we don't reach outside your workspace.

[InDieTasten]

• Stop here, even though require would try /foo/node_modules/boostrap/something.css, we don't reach outside your workspace.

I think you mean /node_modules/bootstrap/something.css in that example.

[penx]

@octref for now, I think your suggestion should satisfy most use cases, however I am pretty sure it will stop working once npm tink is finalised:

https://blog.npmjs.org/post/178027064160/next-generation-package-management
https://www.npmjs.com/package/tink
https://medium.com/@Andrew_Mc/yarn-pnp-npm-tink-and-pnpm-oh-my-720d02221b4b

I think using node module resolution directly would allow easier solutions to things like this, although admittedly it could be hard to ensure vscode's node runtime would always resolve in the same way as the node runtime for the open project.

I also previously mentioned yarn pnp and the yarn portal protocol which could cause issues but I'm not sure how these could be easily solved!

[penx]

I'll try find some time today to modify #79651 to work as @octref has suggested and npm tink compatibility can be considered at a later date. Perhaps @zkat could consider how this might work!

[penx]

I just realised @zkat has left npm and development on tink seems to have stopped, though it is on the roadmap for npm 8

[penx]

@octref I have raised a new PR implementing as you describe above, see #81555

[roblourens]

I don't know much about sass and not sure what I need to verify this. If I follow the steps, I get this error when clicking the link

That's in a .css file. In a .scss file, the import path is not linkified.

[Friksel]

@roblourens Not sure who you target your message at and what you do or don't know about sass etc., but maybe this helps you?:

The path starting with ~ should point to the local node_modules\bootstrap folder inside the projectfolder normally while most developers install bootstrap locally in their project. Not sure if it should officially 'fallback' to a globally installed node-package though, but the path to the bootstrap file in your example is not a global nodes modeles path to my knowledge either. In the message in your screenshot it doesn't look like a project folder to me either, but maybe you have your project inside /private/tmp and called your projectfolder sass?

Open door, but just in case: Is the bootstrap package installed there and does the file /private/tmp/sass/node_modules/bootstrap/scss/bootstrap-grid.scss (could be _bootstrap-grid.scss (with underscore)) exist?

Also as I understand it you see this in a .css file? Not sure what you did there, but @import statements can only be in a scss-file (sass) as that's sass-specific. There's no such thing in css and it wouldn't make sense to import a sass file from a css file either. But probably I just don't get what you mean?

If the import path in the scss-file is not 'linkified' it sounds like something is not working as expected, as normally @import-paths are linkified in scss files. Like so:

Does this help?

[roblourens]

I'm asking @aeschli what I need to set up to verify that it was fixed. And CSS imports do exist. But when I have a workspace with a sass file and an import like

and node_modules/bootstrap/scss/bootstrap-grid.scss does exist, but the import is not linkified.

[JacksonKearl]

Seeing they do linkify in .css, but not .scss. The linked PR seems to only test for .css.

[penx]

Just checking this on my commit here 06634e1

style.css

@import '~bootstrap/scss/bootstrap-grid'; /* unable to open */
@import '~bootstrap/scss/bootstrap-grid.scss'; /* works */
@import '../node_modules/bootstrap/scss/bootstrap-grid'; /* unable to open */
@import '~bootstrap/scss/bootstrap-grid'; /* unable to open */
@import '../node_modules/bootstrap/scss/utilities/sizing'; /* unable to open */

style.scss

@import '~bootstrap/scss/bootstrap-grid'; // works
@import '~bootstrap/scss/bootstrap-grid.scss'; // works
@import '../node_modules/bootstrap/scss/bootstrap-grid'; // works
@import '~bootstrap/scss/bootstrap-grid'; // works
@import '../node_modules/bootstrap/scss/utilities/sizing'; // works

All of the above, I think, is intended behaviour as I wouldn't expect a css file to import an scss file using scss filename resolution (but if for some reason it directly linked to the file then it links through fine).

(edit - if anything is incorrect here it's that the css file shouldn't underline the invalid links?)

@JacksonKearl @roblourens what versions/commits are you testing against?

[penx]

here's a test project (make sure to run yarn from the root first)

https://github.com/penx/vscode-css-link

[roblourens]

I feel like I'm missing a setting or an extension or something. This is on today's insiders.

[penx]

@roblourens and the files definitely exist on disk? i.e. you’ve done a yarn or npm install bootstrap?

I’ll try check the insiders build shortly but may be Monday

[roblourens]

Yeah

[JacksonKearl]

@roblourens that's odd... for me the relative links do properly linkify.

[penx]

I pushed a change to my demo repo as I had the wrong relative path in the root scss files sorry (there are also a second set in a sub directory called styles).

Confirmed for me on Insiders (commit b1ef2bf ) that in the scss file, only the relative links work.

However, on the commit where I raised the PR this was working. Will see if I can figure out what changes.

[penx]

@aeschli looks like you removed the code here? 2e5b082#diff-90971796685fd5e48e8f97b9daf95404L58-L79

[penx]

ok I'm confused now, I wrote a test that should be failing but it's passing.

#99487

@aeschli could you take a look? I'm happy to work on this but could do with some pointers

e.g.

• is linking for scss files being handled elsewhere?
• why is this test passing if it's not working on Insiders?
• what do I need to do to test/ensure this is still working with the web/headless version? (which I think has been the previous issue)

[aeschli]

@penx Yes, I moved the code into the service: https://github.com/microsoft/vscode-css-languageservice/blob/master/src/services/cssNavigation.ts#L273

I broke the node module resolving for scss as scss has some additional resolving resolving on.
Sorry, for that. I pushed a fix, added a test for scss.

If you have any additional any tests, you can add them

• css: https://github.com/microsoft/vscode-css-languageservice/blob/61ed31a6083a6f61f28433d71c188f375d586f0b/src/test/css/navigation.test.ts#L362
• scss: https://github.com/microsoft/vscode-css-languageservice/blob/61ed31a6083a6f61f28433d71c188f375d586f0b/src/test/scss/scssNavigation.test.ts#L205
  I will eventually remove the (now duplicated) tests from
  https://github.com/microsoft/vscode/blob/master/extensions/css-language-features/server/src/test/links.test.ts

[Friksel]

And CSS imports do exist.

Yes I know. But it doesn't make sense to import sass from css.