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

Fix types, better exports and use tsup for build, remove parcel #6461

Merged
merged 12 commits into from
Nov 4, 2024

Conversation

sneridagh
Copy link
Member

@sneridagh sneridagh commented Nov 3, 2024

We are having problems in some setups in macos where parcel errors with a segmentation fault and we found also some weird errors related with it in pnpm lock, where the lock keeps updating because of it (apparently, their entries in the lock keep cycling infinitely). In addition, the resultant code has some weird names with a looong hash that end up in the dev tools. Then I also learned that tsup is more pure with their outputs and it was the recommended setup, see:

https://www.totaltypescript.com/how-to-create-an-npm-package

Totally worth it from top to bottom.

I also fixed the remaining types that forced us to ||true in the build output. So overall this PR improves the current situation, in a non-breaking way.

@sneridagh sneridagh requested a review from pnicolli November 3, 2024 23:26
Copy link

netlify bot commented Nov 3, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 4ac8e2e
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/6729072b1deec6000843f296

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

Could you explain more about the switch from parcel to tsup? What's the motivation and are there any tradeoffs?

packages/registry/package.json Outdated Show resolved Hide resolved
@sneridagh
Copy link
Member Author

@davisagli just added some more background in the description.

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Was parcel something the developer directly called? If so, does its replacement merit more than a change log entry?

packages/registry/news/6461.bugfix Outdated Show resolved Hide resolved
packages/registry/news/6461.internal Outdated Show resolved Hide resolved
packages/scripts/news/6461.internal Outdated Show resolved Hide resolved
@sneridagh
Copy link
Member Author

@stevepiercy no, always in automated build processes, but they could see when it failed.

@sneridagh
Copy link
Member Author

BTW, @davisagli @stevepiercy in the next days, probably I will be replacing (if possible) parcel with tsup in all our builds

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

I'm not really qualified to review the Node/Typescript details here. I'm okay with it as long as there isn't an impact for users.

@sneridagh sneridagh merged commit 60440ec into main Nov 4, 2024
68 checks passed
@sneridagh sneridagh deleted the betterploneregistrybuild branch November 4, 2024 19:33
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.

3 participants