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.

Sign up for GitHub

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

Jump to bottom

Output ESM instead of CommonJS #225

Merged
merged 22 commits into from
Nov 3, 2021
Merged

Output ESM instead of CommonJS #225

merged 22 commits into from
Nov 3, 2021

Conversation

SimonSiefke
Copy link
Contributor

@SimonSiefke SimonSiefke commented Oct 16, 2021 •
edited
Loading

this would be a breaking change because of esm output

fixes #215

@sindresorhus
Copy link
Owner

CI is failing

@sindresorhus
Copy link
Owner

I'm willing to merge this now, but I won't release this until TS 4.5 final is out.

@SimonSiefke
Copy link
Contributor Author

yeah, it's a bit unfortunate that ts-node doesn't support typescript-4.5 beta TypeStrong/ts-node#1519

I think it would be compatible, just the semver (>=2.7) doesn't seem to allow beta release (4.5.0-beta)

I've now changed npm install to npm install --force so that ci (hopefully) passes

@SimonSiefke
Copy link
Contributor Author

I also needed to upgrade ava and xo so that they support esm, but I think everything should work now. Sorry that the pr is very big now :(

sindresorhus
sindresorhus reviewed Oct 17, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
SimonSiefke
SimonSiefke commented Oct 17, 2021
View reviewed changes
source/predicates/predicate.ts
message: (_, label, error) => typeof error === 'string'
? `(${label}) ${error}`
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
: error(label),
Copy link
Contributor Author

@SimonSiefke SimonSiefke Oct 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

not sure how to fix these two errors so I disabled them for now

@cspotcode cspotcode mentioned this pull request Oct 18, 2021
Closed
@sindresorhus
Copy link
Owner

CI is failing because of xojs/xo#599

@sindresorhus
Copy link
Owner

Alright. XO is fixed now. Can you fix the tests?

@SimonSiefke
Copy link
Contributor Author

Node 14 CI Coverage is failing because of avajs/ava#2417
Changed nyc to c8 which worked locally

Node 16 CI Tests are failing because of ESM Loader Change in Node 16.12.0 (which was released yesterday), which breaks ts-node
I opened TypeStrong/ts-node#1526

@SimonSiefke
Copy link
Contributor Author

Updated ts-node, I think all should work now.

sindresorhus
sindresorhus reviewed Oct 24, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
@voxpelli
Copy link

I'm looking into why xo fails in this PR as it seems to be related to my PR in xojs/xo#624, though an npm install on a fresh checkout of this PR gives me:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/typescript
npm ERR!   dev typescript@"^4.5.0-beta" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer typescript@">=2.7" from [email protected]
npm ERR! node_modules/ts-node
npm ERR!   dev ts-node@"^10.4.0" from the root project

Did you install using npm install, npm install --legacy-peer-deps or npm install --force @SimonSiefke @sindresorhus?

@voxpelli
Copy link

Posted about the cause of the xojs/xo#624 related issue in that PR.

Though, as a separate issue, I don't think npm install --legacy-peer-deps or npm install --force should be required for ow, so if that's something this PR introduced, then if that can be avoided then that would be great 👍

@sindresorhus sindresorhus changed the title output esm, fixes #215 Output ESM instead of CommonJS Nov 3, 2021
@sindresorhus sindresorhus merged commit 0288f6a into sindresorhus:main Nov 3, 2021
@voxpelli
Copy link

voxpelli commented Nov 3, 2021

👏 🎉

@jonahsnider
Copy link

I'm willing to merge this now, but I won't release this until TS 4.5 final is out.

TypeScript v4.5 has now been released, can this PR be released now @sindresorhus?

@sindresorhus
Copy link
Owner

Unfortunately not: #227 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sindresorhus sindresorhus sindresorhus left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug with esm
4 participants
@SimonSiefke @sindresorhus @voxpelli @jonahsnider