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

monaco-editor vite blocker #200

Open
johnlindquist opened this issue Dec 31, 2022 · 27 comments
Open

monaco-editor vite blocker #200

johnlindquist opened this issue Dec 31, 2022 · 27 comments
Labels
bug Something isn't working

Comments

@johnlindquist
Copy link
Collaborator

johnlindquist commented Dec 31, 2022

  • 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

@johnlindquist johnlindquist added the bug Something isn't working label Dec 31, 2022
@suren-atoyan
Copy link

@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
Copy link

okay, I found this

@johnlindquist
Copy link
Collaborator Author

@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
Copy link

@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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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

@suren-atoyan
Copy link

@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
Copy link
Collaborator Author

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

@suren-atoyan
Copy link

suren-atoyan commented Jan 2, 2023

@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
Copy link

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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

johnlindquist commented Jan 2, 2023

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
Copy link

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
Copy link

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
Copy link
Collaborator Author

"Force load" meant:

  1. cmd+; to Open main prompt
  2. cmd+o to Open editor (broken)
  3. cmd+; to force back to main prompt
  4. 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
Copy link
Collaborator Author

@suren-atoyan
Copy link

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
Copy link

@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
Copy link
Collaborator Author

@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
Copy link

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
Copy link
Collaborator Author

@johnlindquist
Copy link
Collaborator Author

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

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
Copy link

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
Copy link
Collaborator Author

@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
Copy link

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
Copy link
Collaborator Author

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"

🤷‍♂️

electron-vite/vite-plugin-electron-renderer#21 (comment)

@JosXa
Copy link
Contributor

JosXa commented May 24, 2024

I presume this issue can be closed @johnlindquist?

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

3 participants