addImportSpecifier() prevent duplicates
#2081
Conversation
cartogram
changed the title
addImportSpecifier() prevent duplicates
|
@Shopify/web-foundations do you know what the ruby-tests failures in CI are about? |
|
@cartogram Short answer: No :( I poked around a bit last week (so did @bradsokol), but no resolution yet. |
|
@cartogram The build issue on main is fixed. |
|
@atesgoral Thanks for following up! Would I also be able to get a review 🍏 from you or someone on the web-foundations team? |
| @@ -19,5 +19,19 @@ export default function addImportStatement(statement: string | string[]) { | |||
| path.node.body.unshift(ast); | |||
| }); | |||
| }, | |||
| File(path: traverse.NodePath<t.Program>) { | |||
There was a problem hiding this comment.
Could we add a test for this case?
There was a problem hiding this comment.
Good catch! I can't with my current test utilities because the File node is not part of the fixture (they start at Program because they are coming from strings not files). I tried to get around this without any luck.
I suggest we merge without a test for this and I'll come back to it. These utilities are already well tested given how experimental they are, and this particular logic exists to catch the rare edge case that they run this transform on an empty file.
What do you think?
Co-authored-by: Ateş Göral <[email protected]>
Description
This PR prevents duplicate specifiers from being added when using the
addImportSpecifiertransform. If duplicates end up in the source, they cause a syntax error for transforms down the line.Type of change
Checklist