Skip to content
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

build: clean generated type file #14582

Merged
merged 4 commits into from
Oct 12, 2023
Merged

build: clean generated type file #14582

merged 4 commits into from
Oct 12, 2023

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.

@patak-dev patak-dev merged commit fffe16e into main Oct 12, 2023
9 checks passed
@patak-dev patak-dev deleted the cleanup-types branch October 12, 2023 07:13
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