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

Export all missing modules in compat #4006

Merged
merged 14 commits into from
Sep 27, 2024
Merged

Conversation

DavideIadeluca
Copy link
Contributor

@DavideIadeluca DavideIadeluca commented Jul 11, 2024

Fixes #0000

Changes proposed in this pull request:
Export all missing modules with the Compat API and therefore improve the extensibility of the framework.

Reviewers should focus on:
We need to make sure that those exports are adopted in the 2.x equivalent (forum.ts). I suggest that the export registry upgrade step is used just before the first 2.0 beta is released.

Screenshot

QA

  1. Try to extend/override a newly added component from core
  2. Try to extend/override a newly added component from an extension
  3. Try to use a newly added util which is the default export (i.e. fireDebugWarning)
  4. Try to use a newly added util which is a non default export (i.e. fireDeprecationWarning)

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)
  • Related core extension PRs: (Remove if irrelevant)

@imorland imorland added type/extensibility javascript Pull requests that update Javascript code labels Sep 26, 2024
@imorland imorland added this to the 1.8.6 milestone Sep 26, 2024
@imorland
Copy link
Member

@DavideIadeluca what's the progress on this one, are you able to complete this soon-ish?

@DavideIadeluca DavideIadeluca marked this pull request as ready for review September 27, 2024 06:24
@DavideIadeluca DavideIadeluca requested a review from a team as a code owner September 27, 2024 06:24
@DavideIadeluca
Copy link
Contributor Author

@DavideIadeluca what's the progress on this one, are you able to complete this soon-ish?

I just did a round of testing and everything seems to be in order. The CI is failing, but it doesn't appear to be related to the changes I made.

PR is ready for review from my side now.

Copy link
Member

@imorland imorland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this, I'll include it in the upcoming 1.8.6 release

@imorland imorland merged commit e0adf90 into flarum:1.x Sep 27, 2024
272 of 273 checks passed
@imorland imorland deleted the di/compat-exports branch September 27, 2024 06:44
@imorland
Copy link
Member

Ps, would you mind doing the same for 2.x when you have the chance, please?

@DavideIadeluca
Copy link
Contributor Author

Ps, would you mind doing the same for 2.x when you have the chance, please?

Yes, sure I can. I suggested doing it just before the first beta is released in case new components/utils are added/removed etc. and forgotten to be added in forum.ts. But I'm happy to do it anyway in the next coming days.

imorland added a commit that referenced this pull request Sep 28, 2024
@imorland
Copy link
Member

I am reverting this until we can resolve the js typings errors in the CI..

@DavideIadeluca can you possibly take a look at this when you can?

@imorland imorland restored the di/compat-exports branch September 28, 2024 08:08
imorland added a commit that referenced this pull request Sep 28, 2024
@imorland imorland removed this from the 1.8.6 milestone Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code type/extensibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants