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

Use tsc and target ESM only #55

Closed
wants to merge 6 commits into from
Closed

Use tsc and target ESM only #55

wants to merge 6 commits into from

Conversation

jcbhmr
Copy link
Collaborator

@jcbhmr jcbhmr commented Jun 3, 2023

This is a BREAKING CHANGE and would necessitate a version bump.

This PR would...

@mesqueeb
Copy link
Owner

mesqueeb commented Jun 3, 2023

I do like the notion of this PR seeing how much less setup the build step has. It's way cleaner in many ways than what I currently have.

However, before I can merge this i'll need to try and publish a beta version on NPM and double check against https://arethetypeswrong.github.io/?p=is-what If the types are correct then I'd be happy to implement this PR and major upgrade is-what to require node v16.

PS: a reason that I love having the dist folder in the git repo for small util packages like these is that it's really clear to see the exact changes that a build step change brings. That's the reason, and I believe another reason was for DENO, but it's been so long I can't remember fully.

@jcbhmr
Copy link
Collaborator Author

jcbhmr commented Jun 3, 2023

it's really clear to see the exact changes that a build step change brings.

Ideally you wouldn't have to care at all! 🤣 I think that GitHub Actions Artifacts are great for this. You can npm pack and upload the .tag.gz for each commit and BOOM! 💥 you can see everything that you'd ever want as a little preview! But that's overkill in most cases since you can just click this:

image

and open in Codespaces in about 20s 😉

But I do understand that you like co-locating build and source code. A good compromise might be the Artifact thing or some kind of "bundle size checker" or similar?

@jcbhmr
Copy link
Collaborator Author

jcbhmr commented Jun 3, 2023

@mesqueeb You can use npm pack .tar.gz output:

image

Looks good! 👍 Targetting ESM only means Node.js v12+. Node.js LTS is 16 which Ends in 3 months and 1 week (11 Sep 2023) https://endoflife.date/nodejs

@jcbhmr jcbhmr self-assigned this Jun 3, 2023
jcbhmr added a commit that referenced this pull request Jun 3, 2023
@jcbhmr jcbhmr mentioned this pull request Jun 3, 2023
4 tasks
@jcbhmr jcbhmr marked this pull request as draft June 3, 2023 23:49
mesqueeb pushed a commit that referenced this pull request Jun 6, 2023
* Create all files

* Split everything into its own file

* Use those individual files in index.ts

* prettier

* Add imports that I forgot 🤣

* Use explicit .js suffix for compat with #55
@jcbhmr
Copy link
Collaborator Author

jcbhmr commented Jun 6, 2023

Closing in favor of stacking a PR on top of #74

@jcbhmr jcbhmr closed this Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants