monaco-editor vite blocker

johnlindquist commented 1 year ago

I'm porting Script Kit from webpack -> vite.
I followed the guide Vite guide w/ imports, self.Monaco, loader.config, etc

Everything works great in dev, but after I build, the app silently breaks. I think it's from trying to load the typescript worker, but I'm lost.

Btw, here's the relevant "vite" branch and "editor.tsx" file:

https://github.com/johnlindquist/kitapp/blob/vite/src/components/editor.tsx#L25

I'm guessing it's something I'd have to change in my "vite.config.ts", but I've tried everything I can think of and I'm lost:

https://github.com/johnlindquist/kitapp/blob/vite/vite.config.ts

Here's a video showing the bug in the production build: https://youtu.be/Lmusf_1stsQ

Here's a production build from the "vite" branch you should be able to try it on: https://github.com/johnlindquist/kitapp/releases/tag/v1.41.96

suren-atoyan commented 1 year ago

@johnlindquist do we have a guide on how to run kitapp locally? I tried:

1) clone the repo 2) switch to the vite branch 3) change local node version to make it compatible with 16.17.1+ 4) npm i 5) npm run dev

I guess there is something else 😄

suren-atoyan commented 1 year ago

okay, I found this

johnlindquist commented 1 year ago

@suren-atoyan did you get the setup working?

The easiest way would be to install the main release (it will set up the ~/.kit, ~/.knode, and ~/.kenv):

https://github.com/johnlindquist/kitapp/releases/tag/v1.41.96

Then "npm i" and "npm run dev" should work.

Again, it only breaks after "npm run build"

Sidenote: The whole point of switching to vite is to make the dev setup process easier then move everything to a monorepo 😅

suren-atoyan commented 1 year ago

@johnlindquist I could manage to setup it up, now I run it on dev, and also I can debug it. It took some time due to a lack of docs though.

Could you please tell me how I should build it and test the build version?

johnlindquist commented 1 year ago

@suren-atoyan

I use the "m1" command from package.json:

"m1": "npm run build && electron-builder --mac dir --arm64 -c.mac.identity=null --publish never",

My final command from the terminal looks like: npm run m1 && open release/mac-arm64/Kit.app

You can remove the "--mac and --arm64" if you're not on an intel mac: "prod": "npm run build && electron-builder --publish never",

npm run prod && open release/**your-os-and-architecture-here**/Kit.app

johnlindquist commented 1 year ago

Also, after a build, you can access dev tools from the menubar icon->Debug-> Open dev tools

suren-atoyan commented 1 year ago

@suren-atoyan

I use the "m1" command from package.json:

"m1": "npm run build && electron-builder --mac dir --arm64 -c.mac.identity=null --publish never",

My final command from the terminal looks like: npm run m1 && open release/mac-arm64/Kit.app

You can remove the "--mac and --arm64" if you're not on an intel mac: "prod": "npm run build && electron-builder --publish never",

npm run prod && open release/**your-os-and-architecture-here**/Kit.app

that's what I do even in dev mode

johnlindquist commented 1 year ago

"m1" creates production "Kit.app". Is that not what you're seeing?

suren-atoyan commented 1 year ago

@johnlindquist I spent a good amount of time on this thing, so let me share my findings.

First of all, to use monaco we have two main options; either ESM where you want to do import monaco from 'monaco-editor' or an already compiled version where you load monaco through its loader.

For the first option, you need a special treatment, if it's webpack you need an additional plugin if it's vite you have to import workers and set up MonacoEnvironment. I struggled with these options quite a lot yesterday. I faced the issues you've shown in your video; couldn't understand what was going on, to be honest - it was really strange. It doesn't load any other worker than editorWorker - feels like somewhere it leads to an error which causes the main screen to crash.

The second option should have been working because it's less magic and we do not depend on the framework. For that, first, you need to copy the monaco-editor into your dist or assets (I choose dist). So, uncomment this, but make 'assets/monaco-editor' to 'dist/monaco-editor'. Next, in App.tsx we need something like this, in our case, it should be:

function ensureFirstBackSlash(str: string) {
  return str.length > 0 && str.charAt(0) !== '/' ? `/${str}` : str;
}

function uriFromPath(_path: string) {
  const pathName = path.resolve(_path).replace(/\\/g, '/');
  return encodeURI(`file://${ensureFirstBackSlash(pathName)}`);
}

const vs = uriFromPath(path.join(__dirname, '../dist/monaco-editor/dev/vs'));
// note that it's "monaco-editor/dev" or "monaco-editor/min", but not "monaco-editor/esm"

loader.config({
  paths: {
    vs,
  },
});

This should have been working, but your setup refuses to load anything with file:/// even though you turned off webSecurity here. I see you use electron-vite-vue which maybe does change those configs under the hood; I am not sure.

To easily check the local file restriction, in VSCode go to your dist folder, open monaco-editor/dev, and do Copy Path in dev folder, in my case it's /Users/surenatoyan/projects/kitapp/dist/monaco-editor/dev/vs, you can open it in your browser to check if it works. If so, configure loader:

// this is temporary
loader.config({
  paths: {
    vs: `/Users/surenatoyan/projects/kitapp/dist/monaco-editor/dev/vs`,
  },
});

I couldn't spend more time on this, but if you make electron to load local resources this should work.

One more note: If you choose the second option to load monaco, do not forget to remove all direct imports of monaco-editor; like this one import * as monaco from 'monaco-editor'; If you need types, you can do this import * as monacoType from 'monaco-editor/esm/vs/editor/editor.api';

suren-atoyan commented 1 year ago

please check these and let me know about the progress, if it doesn't work, I'll do another iteration at the end of this week. In any case, it should be fixed 🙂

johnlindquist commented 1 year ago

@suren-atoyan Thanks you so much for taking so much time to look into this 🙏

Second Option

I was using that "second option": path/vs approach previously in webpack, but the new vite setup complains about not finding marked.js:

CleanShot 2023-01-02 at 10 57 53

Even though I'm using the min/vs and it successfully loads in loader.js, it seems to want to use esm because of vite's settings. I attempted merging in the esm dir with the vs dir to place the expected files, but then ran into an "unexpected error" around:

export var marked

Since I felt it was getting too "hacky", I decided to put my efforts back towards "option one":

For reference, the relevant code:

const vs =
  process.env.NODE_ENV === 'development'
    ? path.join(process.cwd(), 'node_modules', 'monaco-editor', 'min', 'vs')
    : path.join(__dirname, 'vs');

loader.config({ paths: { vs } });

then in the vite.config.ts:

    electron({
      include: ['app'],
      transformOptions: {
        sourcemap: true,
      },
      plugins: [
        copy([
          {
            from: 'node_modules/monaco-editor/min/vs/**/*',
            to: 'dist/vs',
          },
        ]),
        esmodule({
          include: ['execa', 'nanoid', 'download'],
        }),
      ],
    }),

Continuing with Option One

When I strip out almost all the code around the editor, the workers don't "crash".

I'm not sure yet what the conflict is, but I'm going to keep adding/removing bits until I figure out exactly what is causing the crash.

I have a feeling it's something really small and stupid...

johnlindquist commented 1 year ago

First Load

So it appears that loading the typescript worker:

Doesn't load the first time

Which causes a re-render which:

unloads all my state
disconnects the "ipcRenderer"
editor and everything else are broken

Second Load

but, if I force load the a SECOND TIME:

typescript worker successfully loads
all the state/ipcRenderer can re-connect
The editor begins working

Strangely... It creates a second "monaco-aria-container" in the dom... 🙃

CleanShot 2023-01-02 at 15 58 52

So things are still broken, but at least it seems "solvable" by preloading the editor/workers or something 🤔

@suren-atoyan Does that spark any ideas?

suren-atoyan commented 1 year ago

I tried Option 1 with your config and saw the marked.js error. It's really strange, there is a recent open issue in monaco-editor repo, but I also tried with older versions and no luck yet. I'll try option 2 with your suggested force load tomorrow. BTW; what exactly force load means here? 😄

but at least it seems "solvable"

there is nothing unsolvable 🙂

suren-atoyan commented 1 year ago

just in case, if you try to install another version of monaco-editor (or any other package) be sure that it's not cached by vite (there is node_modules/.vite folder); vite caches everything very aggressively.

johnlindquist commented 1 year ago

"Force load" meant:

cmd+; to Open main prompt
cmd+o to Open editor (broken)
cmd+; to force back to main prompt
cmd+o to Open editor again (now working)

I've been attempting to preload/load the editor before "step 1." so that it might work the first time, but no luck so far.

Thanks for the caching tip! I'm new to vite so I'll make sure to keep an eye on it.

(It's bedtime here, I'll check in my "progress" in the morning)

johnlindquist commented 1 year ago

@suren-atoyan Committed my ugly preload hack 🙃

https://github.com/johnlindquist/kitapp/blob/vite/src/App.tsx

suren-atoyan commented 1 year ago

for some reason enter behaves differently in development vs production, is this intentional? In development enter makes a new line, but in production you have to press Shift+Enter

suren-atoyan commented 1 year ago

@johnlindquist that's still a hack obviously, but we can beautify it a little bit. First, we can move that "magic" part from App.tsx file to a separate SetupMonaco kind of component. Second, I think we do not need setTimout or any random time-based logic, we can remove the MonacoEditor right after it's mounted (onMount will help us). Having the SetupMonaco component can give us an opportunity to move there for example theme definitions (and all other pre-setup operations we need).

I did these things, plus a few other cleanups, here is the PR 🙂

johnlindquist commented 1 year ago

@suren-atoyan Thanks for the PR! ❤️

At least this is forcing me to clean up and organize things a bit (still a lot to do 😅)

Re: "Enter"

I don't know what's going on with "Enter" in prod...

It also causes that "line rendering" in prod which I had to manually disable in options.

I also had to add a hack in the "index.html" since it's causing multiple aria divs.

It's so strange... None of that happens in dev 😥

suren-atoyan commented 1 year ago

I don't know what's going on with "Enter" in prod...

The reason why Enter doesn't work is this guy. You can put a debugger/log here. A quick fix for it you can find here.

johnlindquist commented 1 year ago

@suren-atoyan Fixed it...

https://github.com/johnlindquist/kitapp/blob/vite/vite.config.ts#L53

facepalm

johnlindquist commented 1 year ago

I'm not exactly sure what that plugin is doing to the files that causes this behavior:

nodeIntegration was the default setting from the electron-vite-react starter: https://github.com/electron-vite/electron-vite-react/blob/main/vite.config.ts#L43
I still use nodeIntegration from the main prompt: https://github.com/johnlindquist/kitapp/blob/vite/app/prompt.ts#L111

It seems like an odd setting to have in a plugin since an app can have multiple windows, some with nodeIntegration, others without. I'll chat some more with the plugin author.

Thanks so much for all of your help with this (and your patience with me).

suren-atoyan commented 1 year ago

hey @johnlindquist 👋

Very happy that this is finally solved 😄 I told you there is nothing unsolvable 😄 And just in case, Kitapp is great 🙂

Let me know if you find out more about the issue.

johnlindquist commented 1 year ago

@suren-atoyan I believe the issue stemmed from the plugin converting the monaco-editor imports from esm to cjs causing the monaco Workers to crash when trying to import files.

I'm not quite sure how it "recovered" after the crash with our hacky fixes...

suren-atoyan commented 1 year ago

Your hypothesis makes sense, but yes, really, how it "recovered"?

Could you please give me an update on this issue? Was adding nodeIntegration fixed the problem?

johnlindquist commented 1 year ago

Removing "nodeIntegration" from the vite plugin fixed it.

I completely misunderstood the intent of that setting. It was the default and I assumed I needed it (since I'm using the setting in Electron):

In Electron: "nodeIntegration" means: "allow node in the renderer"

In the vite plugin: It means "change rollup compiler to cjs"

🤷‍♂️

https://github.com/electron-vite/vite-plugin-electron-renderer/issues/21#issuecomment-1373025266

JosXa commented 5 months ago

I presume this issue can be closed @johnlindquist?

script-kit / app