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

feat: switch collab server #4092

Merged
merged 1 commit into from
Oct 24, 2021
Merged

feat: switch collab server #4092

merged 1 commit into from
Oct 24, 2021

Conversation

dwelle
Copy link
Member

@dwelle dwelle commented Oct 23, 2021

No description provided.

@vercel
Copy link

vercel bot commented Oct 23, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/mnZsdAuiwNgTCadqKMMaJJ2938Cq
✅ Preview: https://excalidraw-git-switchcollabserver-excalidraw.vercel.app

@dwelle dwelle merged commit d7cdee3 into master Oct 24, 2021
@dwelle dwelle deleted the switch_collab_server branch October 24, 2021 09:47
zsviczian added a commit to zsviczian/excalidraw that referenced this pull request Oct 24, 2021
@zsviczian
Copy link
Collaborator

@dwelle, I did fetch upstream and built the Obsidian Excalidraw package, but I get an error. If I revert this commit, then it builds OK.
image

zsviczian added a commit to zsviczian/excalidraw that referenced this pull request Oct 24, 2021
@dwelle
Copy link
Member Author

dwelle commented Oct 24, 2021

@zsviczian seems like a missing asset error? Unclear how it can be related to this PR that changes an env variable 🤔

@zsviczian
Copy link
Collaborator

I can just confirm my experience. When I reverted this commit, then build worked again. I’ll try it again tomorrow.

@ad1992
Copy link
Member

ad1992 commented Oct 25, 2021

This file is not involved when we publish the releases and hence @excalidraw/excalidraw-next was not published for this PR as well. @zsviczian have you made some changes in the fork in the way we publish the release ?

@zsviczian
Copy link
Collaborator

@ad1992: I've made changes to src/packages/excalidraw/webpack.prod.config.js to package everything into a single file because that is more efficient in Obsidian. But that should have nothing to do with the error above, since I was executing yarn build in the root directory, and not in the excalidraw package folder.

I kept the changes in my fork to the bare minimum (onDrop, onBeforeTextEdit, onBeforeTextSubmit and rawText field on Text element), otherwise my fork should be equivalent to master.

What sort of changes should I be looking for that may impact publish?

@ad1992
Copy link
Member

ad1992 commented Oct 25, 2021

@ad1992: I've made changes to src/packages/excalidraw/webpack.prod.config.js to package everything into a single file because that is more efficient in Obsidian. But that should have nothing to do with the error above, since I was executing yarn build in the root directory, and not in the excalidraw package folder.

I kept the changes in my fork to the bare minimum (onDrop, onBeforeTextEdit, onBeforeTextSubmit and rawText field on Text element), otherwise my fork should be equivalent to master.

What sort of changes should I be looking for that may impact publish?

Ok, that would be adding the webpack.optimize.LimitChunkCountPlugin({ maxChunks: 1 }) to package into a single file. This shouldn't affect anything. Since the assets are not found, could you make sure after the build command you have actually copied the excalidraw-assets folder to the obsodian plugin?

But that should have nothing to do with the error above, since I was executing yarn build in the root directory, and not in the excalidraw package folder.

You should execute it inside the package folder instead, running in root will try to build excalidraw app as whole.

@zsviczian
Copy link
Collaborator

I always build both the app and the package. I first build the Excalidraw app from root, then build the package from the package folder. In this case I didn't get to step two because I ran into the error building the Excalidraw app.

@ad1992
Copy link
Member

ad1992 commented Oct 25, 2021

I always build both the app and the package. I first build the Excalidraw app from root, then build the package from the package folder. In this case I didn't get to step two because I ran into the error building the Excalidraw app.

why do you need to build the app from root ? You are using your forked version of package right ? So then building app is not needed. yarn install in both root and package is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants