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

Migrate to Vue 2.x #461

Open
cpb8010 opened this issue May 2, 2018 · 24 comments
Open

Migrate to Vue 2.x #461

cpb8010 opened this issue May 2, 2018 · 24 comments
Labels

Comments

@cpb8010
Copy link
Contributor

@cpb8010 cpb8010 commented May 2, 2018

I was trying to include some spiffy new components and found that the project is still on vue 1.0.18 (and the old vuex)

If I started this work, is there any interest in getting this merged?
Is someone already doing this? Has someone tried and failed?

I'm starting work on this on my own fork and wanted to see if there was any traction here.

@klembot

This comment has been minimized.

Copy link
Owner

@klembot klembot commented May 2, 2018

I've looked into it; the main obstacle I see is that Vue 2.x removed $broadcast, $dispatch, and $emit which are used. I'm open to a PR but would want to take a careful eye to make sure no regressions are introduced. Ideally, this migration would include adding tests on the Vue components, but I also realize that adds a lot of work.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented May 2, 2018

@wraybowling

This comment has been minimized.

Copy link

@wraybowling wraybowling commented May 8, 2018

I dove into twinejs for the first time today, and immediately noticed the same thing. @cpb8010 I'll happily assist with the effort.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented May 10, 2018

Hey @wraybowling

I was able to get through most of the migration helper, but I think the start-up process for Vue 2 is different so the mixins/plugins don't load quite the same.
I have the changes here:
cpb8010#1 / https://github.com/cpb8010/twinejs/tree/tx-pass3

If you want to pull it down to take a pass at it.
I'll be muddling around with it as I dig through the docs.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented May 18, 2018

I figured part of it out. Vue 2 doesn't support require syntax the same way and the migration helper missed some of the router and app init changes. I'll post again when I have that sorted.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented May 20, 2018

So I figured that part out, but since $appendTo was removed $mountTo is broken.

8 tests are failing. 4 with CodeMirror and 4 with $mountTo.

cpb8010#2

I think it's close, but I haven't gotten far enough to actually load a story in the application

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented May 28, 2018

Turns out the failing tests were because of the same init issues where it pulled in the version without the template compiler. I haven't figure out enough webpack to make this work without changing the test source.

I took the $appendTo source from Vue 1 to fix $mountTo, but I'm still not sure it works because it doesn't show any default stories

@wraybowling

This comment has been minimized.

Copy link

@wraybowling wraybowling commented May 30, 2018

I wasn't able to get the repo to build.. it may be a little while before I can contribute

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented May 30, 2018

Hey @wraybowling I had some old branches pushed that wouldn't build, the most recent one is https://github.com/cpb8010/twinejs/tree/fix-startup. Let me know if you have specific errors I can help with.

@wraybowling

This comment has been minimized.

Copy link

@wraybowling wraybowling commented Jun 7, 2018

@cpb8010 Okay. I got my setup working. Now to make sense of what you're doing... I've been using the .vue file structure; not as used to seeing it all split up like this.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented Jun 7, 2018

@wraybowling

This comment has been minimized.

Copy link

@wraybowling wraybowling commented Jun 7, 2018

@cpb8010 Where can I learn more about the webpack setup?
Yeah, it's fun using es2017 and .vue files; it's what lured me in to trying Vue. I think you're wise to upgrade one little piece at a time if you understand how to do so.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented Jun 7, 2018

@klembot

This comment has been minimized.

Copy link
Owner

@klembot klembot commented Jun 7, 2018

Just popping in to say that while I could be talked into using .vue files, it would take some convincing. I understand that that is how you're supposed to do it in Vue 2.x, but I am hesitant because

  • Although it is often possible in editors to switch syntax modes inside a single file, I've always found it to be a pain to set up... though I have not tried this in quite a while
  • I worry that we would end up with thousand-line monstrosities for some of the more intricate components

I'm happy to chat more about the migration process and any other decisions you may need to make-- I would rather stay in regular contact as you guys work on this rather than having a big debate at the very end. Keep on truckin 🚚

@klembot

This comment has been minimized.

Copy link
Owner

@klembot klembot commented Jun 7, 2018

p.s. on my todo list has been migrating from NW.js to Electron. Just saying in case you find yourself banging your head against a wall with NW.js.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented Jun 7, 2018

Thanks!

I did briefly look into something like https://simulatedgreg.gitbooks.io/electron-vue/content/en/
so I'll cross that bridge when I come to it!

The farthest I've gotten was just getting a story loaded and it not showing any of the elements on the grid.

@greyelf

This comment has been minimized.

Copy link

@greyelf greyelf commented Jun 7, 2018

migrating from NW.js to Electron

@klembot
Could you explain your reasons for considering this migration?
(I'm not questioning it as I have little experience with either, but I'm looking to learn one of them in greater detail and knowing your reasons may help me choose.)

@klembot

This comment has been minimized.

Copy link
Owner

@klembot klembot commented Jun 16, 2018

At a high level... Electron has more momentum behind it right now thanks to big projects like VS Code and Slack using it, and accordingly working with it has a lot less friction. The other thing I have liked more about it is that it enforces a separation between the "web" part and the part of the app that knows about the filesystem, etc. The Twine code right now patches behaviors inside the web app, which worked OK at first but is messy to debug and/or edit.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented Jul 2, 2018

More updates

After some debugging I found that it's difficult to emulate the existing modal templates' thenable flow without broadcast and dispatch. The modal-dialog to prompt relationship in particular is what is tripping me up. The other part is that the migration helper didn't catch all of the old dispatches and event sections, so things were more broken than they should have been.

If I get more stuck I might end up refactoring to streamline and simplify this flow assuming nothing external is dependent on it.

@klembot

This comment has been minimized.

Copy link
Owner

@klembot klembot commented Jul 5, 2018

Just curious, would you be able to chat about the state of this on the Discord sometime? I'm on the Eastern time zone.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented Jul 6, 2018

@klembot

This comment has been minimized.

Copy link
Owner

@klembot klembot commented Jul 8, 2018

What's your username on Discord? I have a couple friend invites from strangers and I'm not sure which might be you.

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented Jul 29, 2018

Another update on this, I was able to get the modal prompt working by avoiding $mountTo/thenable.
There are another 15 uses of $mountTo which might also be subtly broken, but the workaround I did was to mount and hide the component on the virtual DOM, then trigger showing it with the data it needs over the global event bus. This didn't take long, but it's not the cleanest solution I'd wish worked.

As for the next problem, I don't know if this is what is breaking the story-edit-view. There are lots of errors (including a stack overflow coming from a Vuex action) that I'll need to figure out before making more progress.

https://github.com/cpb8010/twinejs/tree/change-prompt Is my latest branch.

If you made any progress on your experimental work, let me know and I'd also be glad to help there 🔨

@cpb8010

This comment has been minimized.

Copy link
Contributor Author

@cpb8010 cpb8010 commented Nov 18, 2018

So my PR is here: #501

I changed only what I came across, so no big changes like single file components or electron.
Most of it was simple syntax changes, and the functional changes were small except for the $mountTo/thenable stuff mentioned above, which I've completely removed for static components in the parent template. I got really stuck because I was working in Edge and thought that I had broken the play button. Turns out Play with Edge doesn't work in the current twine either! Opera worked fine, so I assume Chrome works as well.

I don't expect 18k lines to be reviewed quickly or accepted easily, so I expect I'll have to keep the branch up to date as other smaller work goes in. Obviously if you have design questions or other requests I'll respond here, the PR, or Discord.

🦃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.