-
Notifications
You must be signed in to change notification settings - Fork 152
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
Flatten entry array & resolve files without extension #157
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Just a couple comments 👍
@@ -114,7 +114,10 @@ export class CssBlocksPlugin | |||
entries = webpackEntry; | |||
} | |||
else if (typeof webpackEntry === "object") { | |||
entries = objectValues(webpackEntry); | |||
function flatten(arr: whatever[]): whatever[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets at least pull this function out to the root of this file. We could move it in to @opticss/util
, but I think thats overkill :) We don't have a utils package in the css-blocks repo yet – I think we want to keep it that way if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
@@ -143,7 +142,7 @@ export class CSSBlocksJSXAnalyzer extends Analyzer<TEMPLATE_TYPE> { | |||
* @param opts Optional analytics parser options. | |||
*/ | |||
public parseFile(file: string): Promise<JSXAnalysis> { | |||
file = path.resolve(this.options.baseDir, file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this change caused a few failing tests – what was the intent here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Totally possible that its a problem with our test harness 🙂 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nah probably caused the failing tests, the problem is it currently doesn't resolve files without extension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
know of a better way?
|
||
function flatten(arr: whatever[]): whatever[] { | ||
return arr.reduce((acc, val) => (acc as whatever[]).concat(Array.isArray(val) ? flatten(val) : val), []) as whatever[]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better signature for this is:
function flatten<T extends whatever>(arr: T[] | T[][]): T[];
Then no casting will be needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with that I'm getting
error TS2349: Cannot invoke an expression whose type lacks a call signature. Type '{ (callbackfn: (previousValue: T, currentValue: T, currentIndex: number, array: T[]) => T): T; (c...' has no compatible call signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok this was a bit tricky. The type of reduce wasn't smart enough to let the type checker work out what was going on. Also, the definition I provided was only two levels of arrays, but the code would handle it recursively. I refactored that function and made it part of our opticss utilities: linkedin/opticss@f104638
@@ -114,7 +114,7 @@ export class CssBlocksPlugin | |||
entries = webpackEntry; | |||
} | |||
else if (typeof webpackEntry === "object") { | |||
entries = objectValues(webpackEntry); | |||
entries = flatten(objectValues(webpackEntry)) as string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the signature below this doesn’t need to be cast.
might want to move the flatten function somewhere else, please tell me where
fixes #153