Skip to content

Fix transform plugin so it can handle more than one matched import sources#103

Merged
layershifter merged 5 commits into
microsoft:mainfrom
statm:fix-transform
Apr 25, 2022
Merged

Fix transform plugin so it can handle more than one matched import sources#103
layershifter merged 5 commits into
microsoft:mainfrom
statm:fix-transform

Conversation

@statm

@statm statm commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

Currently, the transform plugin only keeps tab of the last import statement where the package name (moduleSource) is a match. For example, in:

import { dupImport1 } from 'custom-package';
import { createStyles } from 'custom-package';
import { dupImport2 } from 'another-custom-package';
import { otherImport } from 'another-package';

with config:

{
  "modules": [
    { "moduleSource": "custom-package", "importName": "createStyles" },
    { "moduleSource": "another-custom-package", "importName": "createMyStyles" }
  ]
}

The plugin will only focus on the 3rd import line (import { dupImport2 } from 'another-custom-package';) and then decide that there's no placement needed, thus missing the 2nd import where createStyles actually exists.

This PR fixes this issue by changing the importDeclarationPath into an importDeclarationPaths array, so we don't miss any imports to replace.

@statm statm requested a review from a team as a code owner April 22, 2022 20:33
@github-actions

github-actions Bot commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

📊 Bundle size report

🤖 This report was generated against 29a69fbe8fbae8bc59c1fa1a55856427674a2b8a

Comment thread packages/babel-preset/__fixtures__/duplicated-imports/code.ts Outdated
@layershifter

Copy link
Copy Markdown
Member

There are CI failures on non-existing imports:

__fixtures__/duplicated-imports/code.ts:3:28 - error TS2307: Cannot find module 'another-custom-package' or its corresponding type declarations.

3 import { dupImport2 } from 'another-custom-package';

You can use @ts-expect-error to get rid of them:

// @ts-expect-error This module does not exist, but will be resolved via aliases
import { createStyles } from 'custom-package';

@layershifter layershifter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@statm thanks for the contribution ❤️ I reviewed changes on fresh head and pushed some minor changes.

P.S. There is another problem (#104) that I found it during the review. Please let us know if it affects you.

@layershifter layershifter merged commit 7d4c9b6 into microsoft:main Apr 25, 2022
@layershifter

Copy link
Copy Markdown
Member

Was fixed in @griffel/[email protected].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants