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: use tsc output code in both cjs and esm formats #127

Merged
merged 4 commits into from
Mar 17, 2021

Conversation

Lxxyx
Copy link
Contributor

@Lxxyx Lxxyx commented Mar 17, 2021

Fixes: #126

@Lxxyx Lxxyx requested a review from Skn0tt as a code owner March 17, 2021 03:05
Copy link
Collaborator

@Skn0tt Skn0tt left a comment

Choose a reason for hiding this comment

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

Thanks @Lxxyx for this PR! After looking at the build output of TSDX, I have two questions:

  1. is it possible to not bundle the output together? Bundling should be the responsibility of final application build, and having an unbundled output makes debugging easier for us.
  2. It currently outputs declaration files for each of our source files, allowing people to import superjson/accessDeep or smth. But since the javascript code for that is all in the bundles, that import will break. Is there a way to omit these per-file declaration maps / have them bundled together?

I tried to look them up in the TSDX docs, but couldn't find anything. Maybe you have an idea :D

Also: Do you think it'd be possible to add ESM support without using TSDX? One of the reasons I removed it in #97 is that it does a whole lot of things that I don't understand, and I didn't see the benefits for a small library like SuperJSON.

tsconfig.json Outdated Show resolved Hide resolved
@Lxxyx
Copy link
Contributor Author

Lxxyx commented Mar 17, 2021

@Skn0tt We can do this. Use tsc to output code in both cjs and esm formats, and be forward compatible

{
  "scripts": {
    "build": "npm run build:cjs && npm run build:esm",
    "build:cjs": "tsc",
    "build:esm": "tsc --module es2015 --outDir dist/esm"
  },
  "module": "dist/esm/index.js"
}

@Lxxyx Lxxyx requested a review from Skn0tt March 17, 2021 09:32
Copy link
Collaborator

@Skn0tt Skn0tt left a comment

Choose a reason for hiding this comment

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

We can do this. Use tsc to output code in both cjs and esm formats, and be forward compatible

That'd be great!

@Lxxyx Lxxyx requested a review from Skn0tt March 17, 2021 11:21
@Lxxyx Lxxyx changed the title build: use tsdx build: use tsc output code in both cjs and esm formats Mar 17, 2021
Copy link
Collaborator

@Skn0tt Skn0tt left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot for your work on this ❤️

@Skn0tt Skn0tt enabled auto-merge (squash) March 17, 2021 12:02
@Skn0tt Skn0tt disabled auto-merge March 17, 2021 12:05
@Skn0tt Skn0tt merged commit 75378fd into flightcontrolhq:main Mar 17, 2021
@Lxxyx Lxxyx deleted the fix/esm branch March 17, 2021 12:06
@Skn0tt
Copy link
Collaborator

Skn0tt commented Mar 17, 2021

Published in v1.7.3

@Skn0tt
Copy link
Collaborator

Skn0tt commented Mar 17, 2021

@allcontributors add Lxxyx for code

@allcontributors
Copy link
Contributor

@Skn0tt

I've put up a pull request to add @Lxxyx! 🎉

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.

Incorrect module field in package.json and missing output in ESM format
2 participants