Skip to content

feat: add echo options type to improve DX & use rollup to ship types #378

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

Closed
wants to merge 10 commits into from

Conversation

dammy001
Copy link

@dammy001 dammy001 commented Apr 28, 2023

… files

The bundle size has reduced also.

@dammy001 dammy001 changed the title feat: add echo options type to improve DX && make use of rollup to ship declaration… feat: add echo options type to improve DX & use of rollup to ship types Apr 28, 2023
@dammy001 dammy001 marked this pull request as draft April 28, 2023 20:40
@dammy001 dammy001 changed the title feat: add echo options type to improve DX & use of rollup to ship types feat: add echo options type to improve DX & use rollup to ship types Apr 28, 2023
@dammy001 dammy001 marked this pull request as ready for review April 28, 2023 21:12
@nandi95
Copy link

nandi95 commented May 2, 2023

This is similar to #356 😁

@jessarcher
Copy link
Member

Thanks for the PR!

I'm all for adding the type definitions and eliminating as many any as possible!

There are quite a few additional opinionated changes in this PR, and some bundling changes that have me concerned, given how often stuff like this has broken peoples' apps.

For example, the IIFE changes have me worried that this PR reintroduces the issue solved in #344.

@taylorotwell taylorotwell marked this pull request as draft May 3, 2023 02:18
@taylorotwell
Copy link
Member

Mark as ready for review when ready for me to review again please.

@dammy001
Copy link
Author

dammy001 commented May 3, 2023

Thanks for the PR!

I'm all for adding the type definitions and eliminating as many any as possible!

There are quite a few additional opinionated changes in this PR, and some bundling changes that have me concerned, given how often stuff like this has broken peoples' apps.

For example, the IIFE changes have me worried that this PR reintroduces the issue solved in #344.

@jessarcher i get your point about the IIFE breaking change, I'll revert the declaration part.

@dammy001 dammy001 marked this pull request as ready for review May 30, 2023 08:44
@dammy001
Copy link
Author

@jessarcher please take a look at this again. potential IIFE issue has been reverted

@driesvints driesvints requested a review from jessarcher May 30, 2023 08:56
@taylorotwell taylorotwell marked this pull request as draft May 30, 2023 14:33
@jessarcher
Copy link
Member

@dammy001 There's still an awful lot going on in this PR unrelated to the PR title.

Is it possible to just add the options types without installing extra packages and reconfiguring the entire build process and module set-up?

@driesvints
Copy link
Member

@dammy001 have you seen Jess' questions? Are you still working on this?

@dammy001
Copy link
Author

dammy001 commented Jul 7, 2023

@driesvints I'll work on the changes today.

@driesvints
Copy link
Member

@dammy001 feel free to resend this once you have more time

@driesvints driesvints closed this Mar 5, 2024
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.

5 participants