Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve scss partial uri building #159

Merged
merged 1 commit into from
Jul 4, 2019
Merged

Improve scss partial uri building #159

merged 1 commit into from
Jul 4, 2019

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