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

[pulsar-next] Streamline publishing process to reduce headaches #43

Open
5 tasks done
savetheclocktower opened this issue Feb 23, 2025 · 1 comment
Open
5 tasks done
Labels
bug Something isn't working

Comments

@savetheclocktower
Copy link

savetheclocktower commented Feb 23, 2025

Thanks in advance for your bug report!

  • Have you reproduced issue in safe mode?
  • Have you used the debugging guide to try to resolve the issue?
  • Have you checked our FAQs to make sure your question isn't answered there?
  • Have you checked to make sure your issue does not already exist?
  • Have you checked you are on the latest release of Pulsar?

What happened?

The updated-latest-electron branch of Pulsar (i.e., PulsarNext) had pointed to a custom location for the github package because of voodoo @mauricioszabo had to do in order to get it working in latest Electron (v27 at that time). When we bumped dugite for Linux ARM64 support, PulsarNext didn't get that change as a result. So the github package was failing to load on my Linux ARM64 VM because dugite didn't build a git binary during the Pulsar binary build. (I noticed this while trying to reproduce a different reported bug; since I'm on Apple Silicon, I'm largely restricted to running an ARM64 Linux distro.)

I was able to get it mostly working yesterday by starting from master and recapitulating all of @mauricioszabo’s changes, plus following the directions here (many thanks, @DeeDeeG, for being so fastidious about leaving breadcrumbs!). It mostly works — except it seems to fail to load in dev mode, where it complains about not being able to find its devDependencies. So I must've missed part of the fix.

At any rate: most of the pain here is because of the need to transpile. I know we'll still need to transpile for JSX reasons, but the only other bit of syntax that needs transpilation is import — and I wonder if we can save ourselves some hassle by converting those to CommonJS, as @mauricioszabo did in one of his commits. It's also possible that there's a simple Babel setting we could enable to make this easier, since the nuanced differences between CJS and ESM default exports are well-known and have been solved by other tools.

This package is a giant machine that we mostly don't understand and are mainly just trying to keep the dust off of, but I bet we can make our own lives easier if we spend a little time to understand it slightly better.

Pulsar version

PulsarNext

Which OS does this happen on?

❓ Other(Please specify in the OS details field below)

OS details

all

Which CPU architecture are you running this on?

None

What steps are needed to reproduce this?

  1. Be a maintainer of a branch of Pulsar that uses a newer Electron version
  2. Attempt to bump the github package
  3. Fail
  4. Cherry-pick some commits in some order, then follow instructions unquestioningly
  5. End up with something that only mostly works

Additional Information:

No response

@savetheclocktower savetheclocktower added the bug Something isn't working label Feb 23, 2025
@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 25, 2025

I basically float two patches for every "pretranspiled" release". You found the commit message for one of the patches, the actual pretranspilation.

As for the devDependencies problem, there is also the commit message for the other patch:

meta: Delete atomTranspilers from package.json

This prevents the editor for trying to transpile this package
at startup time, which is unnecessary and redundant on this
"pretranspiled" tag of the package. Should save some CPU cycles.

(Also stops an error message from showing that a devDependency was
missing, and that require()ing it failed.
This tag may potentially be
requested as a tarball, in which case the devDependencies aren't
supposed
to be available. Since we genuinely don't need the
devDependencies in production, allowing the package to load without
errors when the devDependencies aren't present is a good thing.
Can save us whatever file size the devDependencies take up on disk.)

(Since this package's package.json specifies a custom transpiler, Pulsar will try to load that transpiler stack and run it, but those deps are indeed devDependencies and we don't normally have them outside of hacking onthis repo. My two patches are basically recreating a pretranspilation step the old Atom build scripts used to do when building the editor in core repo. Step 1 -- pretranspile. Step 2 -- delete the metadata in package.json that flags the package for transpilation by Pulsar. Credit to Mauricio for figuring out the way to transpile outside of Atom's build scripts. I copy-pasted those babel commands from him verbatim.)

Diff for the "Step 2" part (change needed to avoid the devDependencies error message): 7bb1987

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants