Skip to content

Improve scss partial uri building#159

Merged
octref merged 1 commit intomicrosoft:masterfrom
BPScott:improve-scss-partials
Jul 4, 2019
Merged

Improve scss partial uri building#159
octref merged 1 commit intomicrosoft:masterfrom
BPScott:improve-scss-partials

Conversation

@BPScott
Copy link
Contributor

@BPScott BPScott commented Jul 1, 2019

Currently the regex for checking filenames uses \w which only checks for alphanumeric characters and underscore (_). This leads to @import './foobar' resolving to _foobar.scss while @import './foo-bar resolves to foo-bar, which is confusing.

This PR makes the prefixing the underscore behaviour more consistent:

  • Identifies filenames that contain non-alphanumeric characters,
    'foo-baz' becomes '_foo-baz.scss' and 'foo.bar' becomes '_foo.bar.scss'
  • Identifies if a filename is already prefixed with an _, '_foo' becomes
    '_foo.scss' and '_foo.scss' remains '_foo.scss'

Tangentially related: microsoft/vscode#58204 is an issue that points out that the resolution of scss files isn't trivial because import './foo' could refer to either foo.scss or _foo.scss and we need filesystem access to check which of those files exist. So we're still assuming that people should be prefixing their partial's filenames with underscores, but at least this makes that assumption a little more consistent.

- Identifies filenames that contain non-alphanumeric characters,
  'foo-baz' becomes '_foo-baz.scss',
- Identifies multiple extensions, 'foo.bar' becomes '_foo.bar.scss',
  while 'foo.bar.scss' still becomes '_foo.bar.scss'
- Identifies if a filename is already prefixed with an _, '_foo' becomes
  '_foo.scss' and '_foo.scss' remains '_foo.scss'
@msftclas
Copy link

msftclas commented Jul 1, 2019

CLA assistant check
All CLA requirements met.

@octref
Copy link
Contributor

octref commented Jul 4, 2019

Thanks, I'll take the test case but I'll rewrite the code with a dynamic resolver.

As I have learned from the Cloudflare news, I'll drop the regex. Especially since this operation is one running on each keystroke.

@octref octref merged commit f386bbd into microsoft:master Jul 4, 2019
@aeschli aeschli added this to the August 2019 milestone Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants