Skip to content

Add relative @import support #50

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

Merged
merged 6 commits into from
Oct 14, 2019
Merged

Conversation

k-g-a
Copy link
Contributor

@k-g-a k-g-a commented Oct 4, 2019

Fixes #47 and #48.

Adds support for local @import at-rules for pure css and less stylesheets.
It's done via passing proper from/filename to postcss/less renderers as discussed here.

I've also added couple of tests and postcss-import-sync2 dev dependency for those tests to work.

Unfortunatelly *.scss tests are failing for me with empty received snapshot (even at origin/develop without any modifications) and i've never worked with SCSS, so I did not dare to change anything related to scss. And I had to push with --no-verify to skip husky, but all other tests are passing.

As a side effect of those modifications local class names have changed: the common "file___" prefix is now substituted by real file name (i.e. "test-module__" or "import-module__") - this is reflected in snapshots. As those values are never ment to be used directly by anyone, this change does not seem relative or harmful in any way.

@k-g-a
Copy link
Contributor Author

k-g-a commented Oct 4, 2019

  1. *.scss tests are failing because I could not update snapshot for those (prefix change)
  2. other snapshots have different local class name suffix, but I'm not sure why...

It seems reasonable to enforce local class names without hashes for testing purposes via generateScopeName option of the postcss-icss-selectors plugin.

@mrmckeb
Copy link
Owner

mrmckeb commented Oct 9, 2019

Hi @k-g-a, I see that the tests are failing. If the classnames are changing, it could be related to your local env... I'll be able to dig into this properly on Monday I think.

@lianapache
Copy link
Collaborator

Hi @k-g-a, I updated snapshots locally and it seems to work fine. Probably, as @mrmckeb said, the problem is with you local env

Copy link
Collaborator

@lianapache lianapache left a comment

Choose a reason for hiding this comment

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

LGTM

@mrmckeb mrmckeb merged commit 3b3260f into mrmckeb:develop Oct 14, 2019
@k-g-a
Copy link
Contributor Author

k-g-a commented Oct 14, 2019

@lianapache thanks! Unfirtunately I had no time to check it myself.
Class name prefixes and .scss snapshot updates are reasonable change (as explained above).
The only difference for which I can't figure out the reason are hashes (in css snapshot, for example). Could those be platform-dependant (i'm on windows)?

It seems to me that we need to add import.module.scss file same as for css/less version and add it to the testFileNames array in DtsSnapshotCreator.test.ts. I could not include those as the part of my initial PR as I could not make scss work.

@mrmckeb
Copy link
Owner

mrmckeb commented Oct 20, 2019

Hi @k-g-a, we've just released v2.0.0 which includes this PR and other changes.

Release notes and migration steps can be found here: https://github.com/mrmckeb/typescript-plugin-css-modules/releases/tag/v2.0.0

Please let me know if you have any other issues or feedback! And thanks for your patience.

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.

Less files importing other less files
3 participants