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

Macro Editor doesn't work in local work spaces - resolving of ui bundle missed path by 1 level #120

Closed
akim-muto opened this issue May 26, 2024 · 12 comments
Labels

Comments

@akim-muto
Copy link
Contributor

Hi, nice to meet you.

I have tried MacroEditor, but when I use flyde in VScode, MacroEditor does not work if I bundle the target tsx file with a web pack, but leave the output file in the local working directory.

If I put the file in VScode's flyde extensions directory and run it, it works.

Is there an easy way to get MacroEditor to work with the bundled files in the working directory at this time?

@GabiGrin
Copy link
Contributor

GabiGrin commented May 27, 2024

Hey @akim-muto
First kudos on trying to get that working! Macro UI is far from well-documented :)

Can't reproduce - see video - https://www.youtube.com/watch?v=PfogEynJ3hI

Can you share a small repro repo?

@GabiGrin
Copy link
Contributor

@akim-muto I fixed the link to the video :)

@akim-muto
Copy link
Contributor Author

I very much appreciate your thoughtful response, which even includes a video.

I create minimal repository for reproduce macroeditor doesn’t work in local.
I hope this helps you.

Here are the steps to reproduce it.

1.clone repository
git clone https://github.com/project-coc/Flyde-bMacroEditor.git

2.install dependency
pnpm install
※I can use bun.

3.build with webpack
pnpm run build

4,you can check local macroeditor doesn’t work

5.set flyde stdlib/dist path in vscodeextinsion dir to test/test_config.json

6.install macroeditor to stdlib in vscodeextinsion
pnpm run ins-vscexdir

7.restart vscode

8.can work in this case.
スクリーンショット 2024-05-28 052150

Thank you!

@GabiGrin
Copy link
Contributor

Thanks for the repo!

I don't know why yet, but changing editorComponentBundlePath to ../../dist/ui/Macro.js makes it work!
Screenshot 2024-05-28 at 11 15 39

I found it by:

  1. reloading the extension (to make sure we're in a clean state)
  2. loading a flow in your repo
  3. opening up dev tools in VSCode
  4. could see the wrong path
Screenshot 2024-05-28 at 11 15 02

Also, with the override of node library.js - I understand what you're trying to do now :) you're trying to extend the library on the right. That's something I want to allow users to do themselves. Your hack is impressive! love the resourcefulness :)

I went ahead and added supporting this as an issue here - #121. Is helping out and directly contributing to Flyde something that interests you?

PS: node-inst-vscdir.flyde looks wild :) assuming it was mainly to get a feeling of Flyde, but I'd inline code / new custom nodes for many things there to ensure it's saner to maintain

@akim-muto
Copy link
Contributor Author

Hi!

Thank you! I could confirm the same information here.
I got lost in the use of devtool and it ate up a lot of my time lol

Also I wanted the function to add it to the node library, but the hack and node-inst-vscdir.flyde were largely a stopgap measure to see if there was a problem with the MacroEditor bundle itself, or if it was a path or dependency issue.
As for node-inst-vscdir.flyde, I wanted to use the flyde functionality as you suggested.

I have a strong interest in contributing directly to Flyde.
It's a tool that I hope to use for a long time if all goes well.

However, I have physical or mental health issues that make it difficult for me to guarantee a quick response or a stable response. So my direct contribution may be limited.

@GabiGrin
Copy link
Contributor

GabiGrin commented May 28, 2024 via email

@akim-muto
Copy link
Contributor Author

Oh, I closed it becouse got a breather , but this behavior is a bug, even though it works one way or another.

I know it's not a priority, but maybe someone else will get caught up in the same thing, so do you want to manage the bug in this issue? Do you want to make a new issue?

@GabiGrin
Copy link
Contributor

@akim-muto Thanks, yeah I agree it's confusing behavior. I investigated it, and this is the culprit - https://github.com/flydelabs/flyde/blob/main/resolver/src/resolver/resolve-dependencies/macro-node-to-definition.ts#L24
importPath is the full file path, including the file name, i.e. /projects/src/macro.flyde.ts, so ../dist will result in /projects/src/dist/macro.flyde.ts

It might be confusing, I agree, but wondering if this should be changed or just documented. WDYT?

@akim-muto
Copy link
Contributor Author

Hmm, I think should fix it unless it's very hard to fix.
From what little I've seen, it's simply.

const editorComponentPath = join(
    importPath,
    “../”,
    macro.editorConfig.editorComponentBundlePath
);

It's not the behavior most people expect, and unfortunately all users can't be expected to read through the documentation from cover to cover when there is a problem😢

@GabiGrin
Copy link
Contributor

GabiGrin commented May 29, 2024 via email

@akim-muto
Copy link
Contributor Author

Since we're at it, I'll try PR if there are no problems with the code changes.
It might take some time to prepare the environment for development flyde and to prepare for confirm behavior.
It's a feature that is not yet well documented, and Do you think it doesn't needs to be done so urgently?

I'll also try to document the process of preparing the environment, which will be the windows 10 version.

@GabiGrin GabiGrin changed the title Macro Editor doesn't work in local work spaces Macro Editor doesn't work in local work spaces - resolving of ui bundle missed path by 1 level May 29, 2024
@GabiGrin
Copy link
Contributor

@akim-muto awesome! it should be pretty straightforward - clone, pnpm i and pnpm start
You can ping me at discord as well if you're stuck and need a faster answer https://www.flyde.dev/discord/

Regarding the docs, are you asking whether I think this should be documented? or fixed?

Documentation - yes, for sure, there's #88 open for that. Moreover, there's the structured editor for Macros that isn't documented at all - see example - https://github.com/flydelabs/flyde/blob/main/stdlib/src/Lists/Lists.flyde.ts#L88

I see both documenting Macros and fixing this low priority. But given the fix here is really simple (the code you've mention + a quick find and replace on existing stdlib libs), it's a good first PR

@GabiGrin GabiGrin reopened this May 29, 2024
akim-muto pushed a commit to project-coc/flyde that referenced this issue Jun 6, 2024
akim-muto added a commit to project-coc/flyde that referenced this issue Jun 12, 2024
GabiGrin pushed a commit that referenced this issue Jun 13, 2024
---
Co-authored-by: akim-muto <akim_muto@MyComputer>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants