Skip to content
This repository has been archived by the owner on Oct 30, 2020. It is now read-only.

Add orderAlphabetically option #49

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

FinestV
Copy link

@FinestV FinestV commented Oct 12, 2017

We found when using this loader the .d.ts files would change ordering between commits.

This adds an orderAlphabetically option to ensure the ordering remains consistent

@@ -6,13 +6,15 @@ const filenameToInterfaceName = (filename) => {
.replace(/\W+(\w)/g, (_, c) => c.toUpperCase());
};

const cssModuleToTypescriptInterfaceProperties = (cssModuleKeys, indent = ' ') => {
const cssModuleToTypescriptInterfaceProperties = (cssModuleKeys, orderAlphabetically, indent = ' ') => {
cssModuleKeys = orderAlphabetically ? cssModuleKeys.sort() : cssModuleKeys

Choose a reason for hiding this comment

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

sort mutates the current array

var arr = ['b', 'a'];
arr.sort(); // (2) ["a", "b"]
console.log(arr); // (2) ["a", "b"]

what do you think about using slice or spread to avoid mutation of the incoming parameter?
You may also consider moving the sorting logic into a helper function so you DRY

@FinestV FinestV force-pushed the master branch 2 times, most recently from 62ce71e to 3137d46 Compare October 12, 2017 05:12
@Obi-Dann
Copy link

Obi-Dann commented Sep 5, 2018

@timse what do we need to do get the PR merged ? It's a bit alarming that there were no pull requests merged since 2017. Are you still maintaining the project ?

@erwan-joly
Copy link

seems like a No to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants