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.

Sign up for GitHub

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

Jump to bottom

Allow custom modules.getJSON function #132

Merged
merged 3 commits into from
Jan 22, 2019
Merged

Allow custom modules.getJSON function #132

merged 3 commits into from
Jan 22, 2019

Conversation

lukeed
Copy link
Contributor

@lukeed lukeed commented Oct 21, 2018 •
edited
Loading

Hey~!

As of now, there's no way to access the JSON file of classnames to their outputs. It can only happen thru the getJSON method of postcss-modules, but it's overridden here.

This PR allows a user to define a custom getJSON method, passing it all arguments for them to do as they please. It does not affect the core behavior of the plugin.

Also closes #101

@codecov
Copy link

codecov bot commented Oct 21, 2018 •
edited
Loading

Codecov Report

Merging #132 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
+ Coverage    95.1%   95.14%   +0.03%     
==========================================
  Files           8        8              
  Lines         245      247       +2     
  Branches       84       85       +1     
==========================================
+ Hits          233      235       +2     
  Misses         12       12
Impacted Files Coverage Δ
src/postcss-loader.js 95.18% <100%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d1ec84...d87d015. Read the comment docs.

@lukeed
Copy link
Contributor Author

lukeed commented Oct 23, 2018

Hey @egoist, sorry for ping, but have you had a chance to look at this?

Thanks!

@lukeed lukeed mentioned this pull request Oct 24, 2018
Merged
@flying-sheep flying-sheep mentioned this pull request Oct 24, 2018
Closed
@flying-sheep
Copy link
Contributor

duplicate of #127

@lukeed
Copy link
Contributor Author

lukeed commented Oct 24, 2018

Ah, yes @flying-sheep. If you take the test updates from here, I'll close this. I would also ensure that typeof options.modules.getJSON === 'function'

flying-sheep
flying-sheep suggested changes Oct 26, 2018
View reviewed changes
Copy link
Contributor

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

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

well, since your PR is farther along, you could also just do this and i’ll close mine 😄

src/postcss-loader.js Outdated
modulesExported[filepath] = json
if (typeof options.modules === 'object' && typeof options.modules.getJSON === 'function') {
options.modules.getJSON(filepath, json, outpath)
Copy link
Contributor

Choose a reason for hiding this comment

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

getJSON may be an async function, so you need to return options.modules.getJSON(...) here.

@plesiecki
Copy link

Hi guys,
I would be great if custom getJSON get called before modulesExported[filepath] = json or if you can use return from getJSON to set modulesExported[filepath]

What do you guys think?

@flying-sheep
Copy link
Contributor

flying-sheep commented Oct 31, 2018 •
edited
Loading

that’s not how getJSON works, it would be confusing.

there could be another option though…

@plesiecki
Copy link

Another option sounds good

@egoist egoist merged commit 5faa7a4 into egoist:master Jan 22, 2019
@benkeen
Copy link

benkeen commented Jan 23, 2019

Thanks for merging this @egoist! Look forward to the next release.

@lukeed lukeed deleted the fix/getJSON branch January 23, 2019 01:39
@egoist
Copy link
Owner

egoist commented Jan 23, 2019

@benkeen check out 2.0.0 🤗

@flying-sheep flying-sheep mentioned this pull request Jan 23, 2019
Closed
bung87 pushed a commit to bung87/rollup-plugin-postcss that referenced this pull request Dec 3, 2019
@lukeed @egoist
Allow custom modules.getJSON function (egoist#132)
997216e 
Hey~!

As of now, there's no way to access the JSON file of classnames to their outputs. It can only happen thru the [`getJSON` method of `postcss-modules`](https://github.com/css-modules/postcss-modules#saving-exported-classes), but it's overridden here.

This PR allows a user to define a custom `getJSON` method, passing it all arguments for them to do as they please. It does not affect the core behavior of the plugin.

---

_Also closes #101_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flying-sheep flying-sheep flying-sheep requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ability to access file output JSON
5 participants
@lukeed @flying-sheep @plesiecki @benkeen @egoist