search

johnwalley / allotment

A React component for resizable split views
https://allotment.mulberryhousesoftware.com/
MIT License
990 stars 55 forks source link

chore: update export for modern environments#669 #715

Closed pangxiaoli closed 10 months ago

pangxiaoli commented 1 year ago

669

netlify[bot] commented 1 year ago

Deploy Preview for allotment-storybook ready!

Name Link
Latest commit b3ff0bf6a1f5b4ffa21f4932841d2f19e0e34fd7
Latest deploy log https://app.netlify.com/sites/allotment-storybook/deploys/658e35fe6a24c20008150915
Deploy Preview https://deploy-preview-715--allotment-storybook.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 1 year ago

Deploy Preview for allotment-website canceled.

Name Link
Latest commit b3ff0bf6a1f5b4ffa21f4932841d2f19e0e34fd7
Latest deploy log https://app.netlify.com/sites/allotment-website/deploys/658e35fedd1f680008cb6d2a
jmreidy commented 1 year ago

Just confirming that the current package is broken for TSC today: https://arethetypeswrong.github.io/?p=allotment%401.19.3

eduardovidals commented 11 months ago

@johnwalley Can this be merged? Thanks

onecrazygenius commented 11 months ago

any update on this please for types

johnwalley commented 10 months ago

Hi @pangxiaoli. I appreciate the PR and sorry for not getting around to looking at it sooner.

For everyone experiencing issues I do appreciate that it's frustrating.

I've made similar changes to other packages I maintain and it has not been straightforward. Also, I merged a similar change in the recent past to this package which I chose to revert because it caused issues to existing consumers.

I find the following report from one of the redux org's maintainers instructive:

https://blog.isquaredsoftware.com/2023/08/esm-modernization-lessons/

At the very least I'd like to ensure we pass all the checks on: https://arethetypeswrong.github.io/

We'll get something merged in the near future but I want to build some confidence we're not going to break things for existing consumers of the package. Which is difficult because testing this sort of change is a nightmare 😬

johnwalley commented 10 months ago

I've gone ahead and merged this as 0512d71ae804736f7f133f02a6e068556f8f73cb.

pangxiaoli commented 10 months ago

Hi @pangxiaoli. I appreciate the PR and sorry for not getting around to looking at it sooner.

For everyone experiencing issues I do appreciate that it's frustrating.

I've made similar changes to other packages I maintain and it has not been straightforward. Also, I merged a similar change in the recent past to this package which I chose to revert because it caused issues to existing consumers.

I find the following report from one of the redux org's maintainers instructive:

https://blog.isquaredsoftware.com/2023/08/esm-modernization-lessons/

At the very least I'd like to ensure we pass all the checks on: https://arethetypeswrong.github.io/

We'll get something merged in the near future but I want to build some confidence we're not going to break things for existing consumers of the package. Which is difficult because testing this sort of change is a nightmare 😬

Thank you for your response and the valuable insights you've provided. I completely understand the challenges that come with integrating changes, especially when it involves impacting existing consumers.

I've taken note of your suggestions and have ensured that the PR passes all the checks on arethetypeswrong.github.io. Additionally, I've personally tested the changes on the latest version of Vite, and I'm pleased to confirm that everything is functioning as expected.

I appreciate your patience and guidance throughout this process. Your contributions are truly commendable, and I'm grateful for the opportunity to collaborate on improving the package.