SCSS: resolve bare module imports on Windows (#245579) #440
Conversation
|
@microsoft-github-policy-service agree |
.travis.yml
Outdated
| @@ -0,0 +1,30 @@ | |||
| # Continuous Integration for vscode-css-languageservice | |||
| 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 target = links[0].target!.replace(/\\/g, '/'); |
There was a problem hiding this comment.
target is a URI, it should not contain \
src/services/cssNavigation.ts
Outdated
| // 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://) |
There was a problem hiding this comment.
Maybe a use a regex to make that check [
startsWithSchemeRegex = /^\w[\w\d+.-]:/
…solute Adds `schemeImportLinks.test.ts` with four mocha cases covering `http://`, `https://`, `file://`, and `vscode-resource://` URLs
| @@ -422,7 +422,22 @@ 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 startsWithSchemeRegex = /^\w[\w\d+.-]:/; | |||
There was a problem hiding this comment.
Node module resolving should only happen when this.resolveModuleReferences is true.
(only set for LESS and SCSS).
So this needs to go behind the if (this.resolveModuleReferences) {.
I looks like we already call resolveModuleReference there, so this code is not necessary?
| @@ -0,0 +1,46 @@ | |||
| import * as assert from 'assert'; | |||
There was a problem hiding this comment.
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
| @@ -0,0 +1,18 @@ | |||
| import { assert } from 'chai'; | |||
There was a problem hiding this comment.
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
Fix issue #432