Skip to content

build: clean generated type file#14582

Merged
patak-cat merged 4 commits intomainfrom
cleanup-types
Oct 12, 2023
Merged

build: clean generated type file#14582
patak-cat merged 4 commits intomainfrom
cleanup-types

Conversation

@bluwy
Copy link
Member

@bluwy bluwy commented Oct 11, 2023

Description

Following up from #14571

The generated types file is improved a little.

  • Names like Plugin$1 are renamed to something better (previously happen in api-extractor too). The type names can sometimes show up in TypeScript messages which is confusing.

Additional context

Also wonder if we should export the esbuild type, but I left it out for now.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@bluwy bluwy added the p1-chore Doesn't change code behavior (priority) label Oct 11, 2023
Comment on lines +125 to +126
case 'TransformResult$1': return 'esbuild_TransformResult'
case 'TransformResult$2': return 'rollup.TransformResult'
Copy link
Member

@sapphi-red sapphi-red Oct 11, 2023

Choose a reason for hiding this comment

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

I am a bit concerned that these two might suddenly be swapped in one day. For example, if rollup changes its chunking mechanism or renaming algorithm, or our code changes somehow changes the import order.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm true. I guess it would be safer if they aren't using namespaces at all, so the worst case is the confusing names.

Maybe I'll need to find another way to handle this, checking import-by-import perhaps then swapping it out 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Could this be tested somehow to detect if the order was changed so you can keep the current code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Running a typecheck after building the types could work, but we'd need a cover all the possible JS APIs. I'm currently working on a more strict approach that maybe helps with this.

Copy link
Member Author

Choose a reason for hiding this comment

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

The last commit should be stricter now. Unfortunately it's a little more code.

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

Labels

p1-chore Doesn't change code behavior (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants