DEV: convert chat components to .gjs #24260
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chancancode
added a commit
to chancancode/ember-codemod-template-tag
that referenced
this pull request
Nov 7, 2023
Hardcoded a bunch of thins to make it work with Discourse – the chat plugin to be specific. We have a bunch of custom resolver rules that needs to be ported over to make this work. The overall strategry should be generalizable. We have a bunch of custom resolver logic that needs to be ported over, the average app can probably try to share code with the Embroider resolver – or use the Resolver from `@embroider/core` directly with `.resolver.json`. See discourse/discourse#24260 for how this code was used in context.
This was referenced Nov 7, 2023
chancancode
added a commit
to chancancode/ember-codemod-template-tag
that referenced
this pull request
Nov 7, 2023
Hardcoded a bunch of thins to make it work with Discourse – the chat plugin to be specific. We have a bunch of custom resolver rules that needs to be ported over to make this work. The overall strategry should be generalizable. We have a bunch of custom resolver logic that needs to be ported over, the average app can probably try to share code with the Embroider resolver – or use the Resolver from `@embroider/core` directly with `.resolver.json`. See discourse/discourse#24260 for how this code was used in context.
chancancode
force-pushed
the
chat-gjs-codemod
branch
from
deb3085
to
09536ce
Compare
|
Updated and rebased for #24189 |
|
I would prefer to merge this next week, and you might have other conflicts tomorrow or friday. That's awesome though 🚀 |
chancancode
force-pushed
the
chat-gjs-codemod
branch
from
09536ce
to
4dab6bc
Compare
chancancode
added a commit
to chancancode/ember-codemod-template-tag
that referenced
this pull request
Nov 7, 2023
Hardcoded a bunch of thins to make it work with Discourse – the chat plugin to be specific. We have a bunch of custom resolver rules that needs to be ported over to make this work. The overall strategry should be generalizable. We have a bunch of custom resolver logic that needs to be ported over, the average app can probably try to share code with the Embroider resolver – or use the Resolver from `@embroider/core` directly with `.resolver.json`. See discourse/discourse#24260 for how this code was used in context.
|
Had to back out |
Pretty sure I can keep the pile of hacks working for at least one week :D I can probably make it work with most of the code by then, maybe we just pick a day on the calendar to watch the codemod run and land a single giant commit across the codebase o_0 |
|
Tests are passing now, so pretty sure I have fixed the problem in the codemod, should be good to run it again whenever |
Yes Im fine with that, chat has a lot of tests too, so I'm not too afraid of breaking everything. And I will manually test your PR too. Let's try to aim for next tuesday if that's ok for you, I will warn you if there's some delay. |
|
This is good to rebase and review. Ping me when you are ready, and I will review/test it 👍 |
Some of these files are quite small, and if we rename them in the same commit where we inlined the template, Git may choose to see them as different files. This commit forces Git to recognize the rename, which will preserve the lineage of the refactored files.
chancancode
force-pushed
the
chat-gjs-codemod
branch
from
4dab6bc
to
4b62673
Compare
|
@jjaffeux just re-ran the codemod against the latest main branch! Some of these can be fixed easily with some modification to the pre-conversion code (or some improvements to the codemod), but since this PR is big enough as it as, I think we should leave those for another time and just focus on the mechanically convertible ones here. For the merging logistics, if we want to preserve the Git history lineage, we would want to ensure the first commit makes it into the main branch on its own, and everything else after that can be squashed. Otherwise, squashing the whole PR is fine too, I think on some smaller files Git would recognize it as a delete and add, rather than rename, so when, etc, |
|
Agreed on just getting the big PR out of the way ASAP, and we can look at remaining files after. Having this around for too long will just cause merging issues. Im looking at it right now |
jjaffeux
approved these changes
Nov 14, 2023
|
I confirmed the spec failure is a flakey and not related, green locally. Also tried the full PR locally and couldn't find any issue. Let's go 🚀 Thanks! |
- convert chat/channels-list -> gjs - convert chat/chat-channel-archive-status -> gjs - convert chat/chat-channel-card -> gjs - convert chat/chat-channel-leave-btn -> gjs - convert chat/chat-channel-metadata -> gjs - convert chat/chat-channel-preview-card -> gjs - convert chat/chat-channel-status -> gjs - convert chat/chat-channel-unread-indicator -> gjs - convert chat/chat-composer-dropdown -> gjs - convert chat/chat-composer-message-details -> gjs - convert chat/chat-composer-upload -> gjs - convert chat/channel -> gjs - convert chat/header -> gjs - convert chat/back-link -> gjs - convert chat/channel-title -> gjs - convert chat/close-button -> gjs - convert chat/full-page-button -> gjs - convert chat/left-actions -> gjs - convert chat/right-actions -> gjs - convert chat/toggle-expand-button -> gjs - convert chat/index -> gjs - convert chat/chat-emoji-avatar -> gjs - convert chat/chat-mention-warnings -> gjs - convert chat/chat-message-in-reply-to-indicator -> gjs - convert chat/chat-message-separator-date -> gjs - convert chat/chat-message-separator-new -> gjs - convert chat/chat-message-text -> gjs - convert chat/chat-message-thread-indicator -> gjs - convert chat/chat-notice -> gjs - convert chat/chat-notices -> gjs - convert chat/chat-replying-indicator -> gjs - convert chat/chat-side-panel-resizer -> gjs - convert chat/chat-side-panel -> gjs - convert chat/chat-skeleton -> gjs - convert chat/chat-upload-drop-zone -> gjs - convert chat/chat-upload -> gjs - convert chat/chat-user-display-name -> gjs - convert chat/export-messages -> gjs - convert chat/button -> gjs - convert chat/separator -> gjs - convert chat/avatar -> gjs - convert chat/error -> gjs - convert chat/info -> gjs - convert chat/left-gutter -> gjs - convert chat/archive-channel -> gjs - convert chat/channel-summary -> gjs - convert chat/delete-channel -> gjs - convert chat/edit-channel-name -> gjs - convert chat/move-message-to-channel -> gjs - convert chat/thread-settings -> gjs - convert chat/toggle-channel-status -> gjs - convert chat/mention_without_membership -> gjs - convert chat/scroll-to-bottom-arrow -> gjs - convert chat/item -> gjs - convert chat/unread-indicator -> gjs - convert chat/user-card-button -> gjs - convert chat/full-page-chat -> gjs - convert chat/reviewable-chat-message -> gjs - convert chat/chat -> gjs - convert chat/toggle-channel-membership-button -> gjs - convert chat/chat-preferences -> gjs
davidtaylorhq
force-pushed
the
chat-gjs-codemod
branch
from
4b62673
to
8fac33c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Discourse.mov
This converts most of the components in chat to use
.gjswith proper imports. This is designed to be reviewable commit-by-commit..jsfiles into.gjswithout changing their content. Some of these files are quite small – if we do the rename and inlining of the template in the same commit, Git may not recognize that as a rename as the resulting files are too different. This forces Git to track that as a move/rename, thus preserving the lineage/history..gjsand the.hbsfile. This is so that you can easily tell what actually changed in the template's content..hbsfiles.After review, we can probably squash the individual commits + the final cleanup commit if we want, but to keep the Git history in tact we will want to at least keep the rename commit separate.
This is converted by a custom codemod built off of what @IgnaceMaes started in #24162. See: IgnaceMaes/ember-codemod-template-tag#8 This can be further tweaked to convert components in the rest of the app.
It should be possible to convert most of the skipped files with some light refactoring in a forthcoming PR. This first batch is just those that can be converted mechanically without intervention.