berry icon indicating copy to clipboard operation
berry copied to clipboard

refactor: Deprecate esbuild-plugin-pnp

Open RDIL opened this issue 3 years ago • 4 comments

What's the problem this PR addresses? esbuild v0.15.0 brings native support for pnp, so this PR deprecates the plugin and updates dependencies within the monorepo.

See also: https://github.com/evanw/esbuild/pull/2451

How did you fix it? Updated versions, removed the plugin.

Checklist

  • [X] I have set the packages that need to be released for my changes to be effective.

Note: I wasn't entirely sure what should be released for this, feel free to give any suggestions as to what should be major/minor/patch/declined.

  • [X] I will check that all automated PR checks pass before the PR gets reviewed.

RDIL avatar Aug 10 '22 16:08 RDIL

Blocked until https://github.com/evanw/esbuild/pull/2453 is released.

merceyz avatar Aug 10 '22 16:08 merceyz

@merceyz is that why the CLI build is failing?

fatal error: too many writes on closed pipe

goroutine 6 [running]:
runtime.throw({0x93bec, 0x1e})
        runtime/panic.go:1047 +0x3 fp=0x83dee0 sp=0x83deb8 pc=0x122d0003
os.sigpipe()
        runtime/os_js.go:143 +0x2 fp=0x83def8 sp=0x83dee0 pc=0x139d0002
os.epipecheck(...)
        os/file_unix.go:195
os.(*File).Write(0x80c020, {0x5fa5fc0, 0x21, 0x40})
        os/file.go:183 +0x27 fp=0x83df80 sp=0x83def8 pc=0x15e70027
main.runService.func1()
        github.com/evanw/esbuild/cmd/esbuild/service.go:114 +0xa fp=0x83dfe0 sp=0x83df80 pc=0x1e9e000a
runtime.goexit()
        runtime/asm_wasm.s:401 +0x1 fp=0x83dfe8 sp=0x83dfe0 pc=0x13ec0001
created by main.runService
        github.com/evanw/esbuild/cmd/esbuild/service.go:108 +0x13

goroutine 1 [chan receive]:
runtime.gopark(0xa9328, 0x975d98, 0xe, 0x17, 0x2)
        runtime/proc.go:363 +0x27 fp=0x861980 sp=0x861958 pc=0x12540027
runtime.chanrecv(0x975d40, 0x86fad0, 0x1)
        runtime/chan.go:583 +0x7c fp=0x861a08 sp=0x861980 pc=0x1065007c
runtime.chanrecv1(0x975d40, 0x86fad0)
        runtime/chan.go:442 +0x2 fp=0x861a30 sp=0x861a08 pc=0x10630002
syscall.fsCall({0x85fab, 0x4}, {0x86fba8, 0x5, 0x5})
        syscall/fs_js.go:520 +0x13 fp=0x861b00 sp=0x861a30 pc=0x15850013
syscall.Read(0x0, {0x8da000, 0x4000, 0x4000})
        syscall/fs_js.go:388 +0xb fp=0x861c00 sp=0x861b00 pc=0x1581000b
internal/poll.ignoringEINTRIO(...)
        internal/poll/fd_unix.go:794
internal/poll.(*FD).Read(0x832060, {0x8da000,

RDIL avatar Aug 10 '22 17:08 RDIL

IMO we should first release a new version of @yarnpkg/esbuild-plugin-pnp that checks the version of ESBuild and:

  • if >=0.15.1 (.1 since .0 has some issues that will be fixed by https://github.com/evanw/esbuild/pull/2453), throws an error telling the user to remove the plugin since PnP support is now builtin
  • else, shows a warning telling the user that newer versions of ESBuild now have builtin PnP support

paul-soporan avatar Aug 10 '22 19:08 paul-soporan

is that why the CLI build is failing?

@RDIL I checked but no, seems to be something else, reported upstream in https://github.com/evanw/esbuild/issues/2458

merceyz avatar Aug 11 '22 07:08 merceyz

I can now successfully build and run Yarn on both Windows and Linux with [email protected] however the build is a lot slower, yarn build:cli on master takes 12s 111ms while in this PR it takes 26s 975ms. @evanw in https://github.com/evanw/esbuild/blob/a6c42a19ff62a70d118e8e016a302fcac84ed95f/CHANGELOG.md#0150 you mention over a 10x speedup (3.4s to 0.24s), do you still see those improvements?


I'll convert this PR into a draft until it's in a mergeable state.

merceyz avatar Aug 17 '22 13:08 merceyz

@evanw in https://github.com/evanw/esbuild/blob/a6c42a19ff62a70d118e8e016a302fcac84ed95f/CHANGELOG.md#0150 you noticed over a 10x speedup (3.4s to 0.24s), do you still see those improvements?

Yes I do, but I suspect you are running esbuild in the slowest possible way. You are using the WebAssembly version instead of the native version, which is typically 10x slower, and then you are also running esbuild through Yarn, which adds an additional performance overhead.

To demonstrate the performance overhead of the different ways of calling esbuild, I'm building the Yarn CLI using the following command:

esbuild --bundle packages/yarnpkg-cli/sources/cli.ts --platform=node --outfile=out.js

Here's a table of the resulting times (each is the best of three runs, done in a Linux VM):

Approach Time Slowdown
esbuild (native) 0.587 seconds 1x
yarn esbuild (native) 7.104 seconds 12.1x slower
esbuild (wasm) 12.684 seconds 21.6x slower
yarn esbuild (wasm) 19.681 seconds 33.5x slower

evanw avatar Aug 17 '22 14:08 evanw

Thanks for confirming, I tested again because the results I got were a bit odd, now it's consistently 5s 831ms vs 8s 154ms without changing anything so I'm assuming the previously slow runs were something else causing it.

merceyz avatar Aug 17 '22 16:08 merceyz

It seems the failing tests are triggered by tau-prolog defining a global variable the old way, and Esbuild now adding use strict at the top of the generated bundle. I've set alwaysStrict: false which should revert to the old behaviour (no use strict), although it'd be better to get tau-prolog to fix it on their side.

arcanis avatar Sep 02 '22 13:09 arcanis