fix(addStyles): support exports of transpiled transforms (options.transform)

[akki-jat]

What kind of change does this PR introduce?

bugfix

Did you add tests for your changes?
No

Solves #332

Does this PR introduce a breaking change?
No

[jsf-clabot]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #333 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #333   +/-   ##
=======================================
  Coverage   98.43%   98.43%           
=======================================
  Files           4        4           
  Lines          64       64           
  Branches       21       21           
=======================================
  Hits           63       63           
  Misses          1        1

---

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 4217bd1...717601d. Read the comment docs.

[alexander-akait]

Please add tests and we can merge this

[alexander-akait]

Please accept CLA too

[alexander-akait]

One question, why 0? Maybe better default?

[akki-jat]

I first thought of options.transform.default(obj.css);
But then I thought if there are other modules which uses another name other than default, then Object.values(options.transform)[0](obj.css);, will support all of them.

Then decision is yours.

[alexander-akait]

Error: Error: Uncaught [TypeError: Object.values is not a function], Object.values is not supported in old browsers

[akki-jat]

Yeah, you are right. So then we can use Object.keys or you suggest simply options.transform.default(obj.css);

[akki-jat]

@evilebottnawi I replaced Object.values with Object.keys to support old browsers.
But if you still prefer options.transform.default(obj.css); then tell me I will add that.

And as for accepting CLA, I already done that. I don't know why it is not reflecting here.

[akki-jat]

@evilebottnawi can you now take a look?

[michael-ciniawsky]

Where does it currently check and throw within the loader :) ? While it's an valid issue it's debatable if adding a check for this to the loader in particular is really needed here , the style-loader is still kind of a node module (require) and node simply doesn't support ES Modules natively yet. What about adding an note to the README instead warning that one needs to transpile transforms written with ES Modules beforehand?

[akki-jat]

@michael-ciniawsky it's throw error: transform is not a function when export default or exports.default is used instead of module.exports.
Because when we use export default or exports.default, it returns the transform as object (not as a function) and actual transform function wrapped inside its default property

If want to check, I already created a repo. https://github.com/akki-jat/style-loader-bug--332

About the transpile, most of the user place their transform.js file outside the src folder and they default configuration which transpiles only src folder. And It would be irretating to user if we force this to them.

What do you think? Tell me, we will discuss it.

[michael-ciniawsky]

I see and I'm ok with using Object.keys to loop over the exports here so it at least throws correctly, but I still think there should be a note added explaining this and inform devs that in case their transform uses ES Modules they need to transpiled it beforehand

[michael-ciniawsky]

Around the transform option section something like e.g

README.md#transform

> :warning: In case your transform uses ES Module Syntax you need to transpile it, or otherwise...

[akki-jat]

@michael-ciniawsky you are right about the warning.
But what I am trying to say is right now even after the transpile it throws error.

Why throws error even after the transpile?

export default is transpiled in exports.default and it returns the transform as object (not as a function) and actual transform function wrapped inside its default property.

So, meaning this will solve problem that is occurring after transpiling.