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

feat(loader): add functions support for locals #985

Merged

Conversation

yungvldai
Copy link
Contributor

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

I came across an interesting case in which a custom (modified) css-loader exports functions (as named exports). The current implementation uses JSON.stringify, which causes export values of type function ​​to become undefined. This small improvement will make it possible to handle function exports.

Breaking Changes

No braking changes.

Additional Info

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 24, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: yungvldai / name: Vladislav A. Ivanov (6f77727)

@yungvldai yungvldai force-pushed the feat-locals-stringify branch 3 times, most recently from db5d1be to db3ccf3 Compare October 24, 2022 23:54
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Can you add test case?

@yungvldai
Copy link
Contributor Author

yungvldai commented Oct 25, 2022

Wrote unit test. I don't know how to test the integration because I didn't find a css-loader that exports functions in open-source. Probably, we can mock it 🤔

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Can we add an integration test, because unit tests are useless in most of time and don't provide real examples

@yungvldai
Copy link
Contributor Author

Can you provide an example of how can I mock css-loader output? Did not find similar tests in repo.

@alexander-akait
Copy link
Member

why we need mock css-loader? Also you can create small custom loader to provide example of usage, we need just check work, no need to implement all features in loader, only for test

@yungvldai
Copy link
Contributor Author

Hello! Added tests.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Just intresting - do you have a loader with such code generation?

@yungvldai
Copy link
Contributor Author

Yes, I do. I have a fork of css-loader with the support of such code generation, the main idea is to parse BEM entities from classnames and generate ready-to-use functions with all possible mixes & modifiers. I'm currently working on getting this into open source.

@yungvldai
Copy link
Contributor Author

small demo:
demo

@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Base: 90.34% // Head: 90.37% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (c3dca47) compared to base (866abbe).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #985      +/-   ##
==========================================
+ Coverage   90.34%   90.37%   +0.02%     
==========================================
  Files           5        5              
  Lines         829      831       +2     
  Branches      221      222       +1     
==========================================
+ Hits          749      751       +2     
  Misses         70       70              
  Partials       10       10              
Impacted Files Coverage Δ
src/loader.js 89.01% <100.00%> (ø)
src/utils.js 58.82% <100.00%> (+0.99%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@alexander-akait alexander-akait merged commit 65519d0 into webpack-contrib:master Nov 16, 2022
@alexander-akait
Copy link
Member

Thank you 👍 Sorry for delay

@yungvldai
Copy link
Contributor Author

yungvldai commented Nov 16, 2022

Thank you too😊

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.

None yet

2 participants