-
Notifications
You must be signed in to change notification settings - Fork 34
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
Start on experimental esbuild support #681
Conversation
🦋 Changeset detectedLatest commit: 1494857 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🕺 💃 :D :D |
This needs more time in the oven.
This has been collecting dust for too long and I think it would be more productive to chip away at the integration in smaller PRs against the default branch. It seems I've gotten far enough to get skuba to build itself, though there are a tonne of missing features and we probably need to clean up the error/warning output. You're way more experienced with esbuild @samchungy and we'll likely lean on you heavily here 🙂. |
src/global.d.ts
Outdated
// evanw/esbuild#2388 | ||
|
||
declare global { | ||
namespace WebAssembly { | ||
interface Module {} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, forgot one thing: we need to figure out a way to include declaration in skuba's bundle and for TypeScript to pick it up in consuming repos, else you'll get compiler errors when type checking esbuild itself unless you have DOM
types or skipLibCheck: true
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think 1494857 will do it, but this upcoming release probably necessitates a beta or two to ensure everything comes together correctly.
Tbh though I haven't used esbuild without |
src/cli/build/esbuild.ts
Outdated
? 'node' | ||
: undefined, | ||
plugins: [ | ||
tsconfigPaths({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting - is the use of this plugin because of evanw/esbuild#394?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, spot on; I'll include this as a comment. Not super enthused by the v0 plugin but it's the simplest path for now; if we end up building our own plugins for more complex scenarios I reckon we could vendor/improve on this.
Co-authored-by: Sam Chung <[email protected]>
This is very MVPR but I suppose it closes #588, at least from an investigative standpoint.