Skip to content
21 changes: 17 additions & 4 deletions src/services/cssNavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,8 @@ export class CSSNavigation {
const documentFolderUri = dirname(documentUri);
const modulePath = await this.resolvePathToModule(moduleName, documentFolderUri, rootFolderUri);
if (modulePath) {
const pathWithinModule = ref.substring(moduleName.length + 1);
return joinPath(modulePath, pathWithinModule);
const remainder = ref.length > moduleName.length ? ref.slice(moduleName.length + 1) : ''; // skip the '/', bare import
return remainder ? joinPath(modulePath, remainder) : modulePath; // e.g. "@import 'bootstrap';"
}
}
}
Expand All @@ -422,7 +422,20 @@ export class CSSNavigation {
return this.mapReference(await this.resolveModuleReference(target, documentUri, documentContext), isRawLink);
}

const ref = await this.mapReference(documentContext.resolveReference(target, documentUri), isRawLink);
// Treat bare module names (“bootstrap/...”) like sass-loader does
const isBareImport = !target.startsWith('.') // not ./ or ../
&& !target.startsWith('/') // not workspace-absolute
&& target.indexOf(':') === -1; // not a scheme (file://)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe a use a regex to make that check [
startsWithSchemeRegex = /^\w[\w\d+.-]:/


if (isBareImport) {
const moduleRef = await this.mapReference(
await this.resolveModuleReference(target, documentUri, documentContext),
isRawLink);
if (moduleRef) { return moduleRef; }
}

const ref = await this.mapReference(
documentContext.resolveReference(target, documentUri), isRawLink);

// Following [less-loader](https://github.com/webpack-contrib/less-loader#imports)
// and [sass-loader's](https://github.com/webpack-contrib/sass-loader#resolving-import-at-rules)
Expand Down Expand Up @@ -589,4 +602,4 @@ export function getModuleNameFromPath(path: string) {
}
// Otherwise get until first instance of '/'
return path.substring(0, firstSlash);
}
}
18 changes: 18 additions & 0 deletions src/test/scss/scssNavigation-node-modules.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { assert } from 'chai';
Copy link
Collaborator

Choose a reason for hiding this comment

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

please move these into scssNavigation.test.ts, where we have all other tests for node module resolving.
Please use the already existing test framework and style (mocha with suite and test

import { getSCSSLanguageService } from '../../cssLanguageService';
import { TextDocument } from 'vscode-languageserver-textdocument';
import { FileType, getNodeFSRequestService } from '../../nodeFs';
import { URI } from 'vscode-uri';

const ls = getSCSSLanguageService();
const mockFS = getNodeFSRequestService();

describe('SCSS link navigation – node_modules', () => {
it('resolves bootstrap path on Windows', async () => {
const doc = TextDocument.create('file:///c:/proj/app.scss', 'scss', 1,
"@import 'bootstrap/scss/variables';");
const links = await ls.findDocumentLinks2(doc, ls.parseStylesheet(doc), {}, mockFS);
const expected = URI.file('c:/proj/node_modules/bootstrap/scss/_variables.scss').toString();
assert.strictEqual(links[0].target, expected);
});
});
1 change: 1 addition & 0 deletions triggerbuild.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
build