Fix path imports #706
Fix path imports #706
Conversation
gulp/build.js
Outdated
|
|
||
| // Sorry, this is a little hard to read, it was hard for me to figure | ||
| // out :facepalm: | ||
| const exportCode = specifierType === 'ImportSpecifier' ? ` |
There was a problem hiding this comment.
There are three kinds of imports, this covers all three of those cases and most importantly ensures there's a default export in each case.
Codecov Report@@ Coverage Diff @@
## master #706 +/- ##
==========================================
+ Coverage 91.38% 91.42% +0.03%
==========================================
Files 157 157
Lines 2671 2671
Branches 739 739
==========================================
+ Hits 2441 2442 +1
Misses 219 219
+ Partials 11 10 -1
Continue to review full report at Codecov.
|
gulp/build.js
Outdated
|
|
||
| // Sorry, this is a little hard to read, it was hard for me to figure | ||
| // out :facepalm: | ||
| const exportCode = specifierType === 'ImportSpecifier' ? ` |
There was a problem hiding this comment.
I think a switch statement might make this a little more clear.
There was a problem hiding this comment.
I dunno, the switch is pretty ugly too:
let exportCode = '';
switch (specifierType) {
case 'ImportSpecifier':
exportCode = `
import { ${exportName} } from '${specifierPath}';
export default ${exportName};
export * from '${specifierPath}';
`;
break;
case 'ImportDefaultSpecifier':
exportCode = `
import def from '${specifierPath}';
export default def;
`;
break;
case 'ImportNamespaceSpecifier':
exportCode = `
import * as def from '${specifierPath}';
export default def;
export * from '${specifierPath}'
`;
}
| it('should align to bottom if the bottom of the node is below the fold', () => { | ||
| // This test cannot be run anymore because `clientHeight` cannot be mutated | ||
| // and I wasn't able to figure out how to set it | ||
| it.skip('should align to bottom if the bottom of the node is below the fold', () => { |
There was a problem hiding this comment.
clientHeight can't be mutated in jsdom anymore? Is this from a jest update?
There was a problem hiding this comment.
I believe so, I couldn't find an alternative either.
There was a problem hiding this comment.
it was never a great test to begin with... i vote for delete
gulp/build.js
Outdated
|
|
||
| // Sorry, this is a little hard to read, it's effectively a switch | ||
| // statement on specifierType | ||
| const exportCode = specifierType === 'ImportSpecifier' ? ` |
There was a problem hiding this comment.
i think it would be cleaner with our usual
condition
? then
: else
format, esp. if combined with https://github.com/declandewet/common-tags#stripindent
package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "lucid-ui", | |||
| "version": "2.13.0", | |||
| "version": "2.13.1-path-imports.4", | |||
PR Checklist
Unit tests written (commonat minimum)semver-labelsOne core team UX approval