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

Replace forum and admin global compat exports with a Proxy #2488

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

dsevillamartin
Copy link
Member

@dsevillamartin dsevillamartin commented Dec 6, 2020

Refs #2085.

Replaces flarum/flarum-webpack-config#7.

Adds ability for import statements of core & core extensions to include their namespaces (forum, admin, and common).

We'll eventually require extensions use the namespaces in their imports. This PR makes them optional for extensions and allows extensions to continue working once we require them.

Original compat objects are left intact & do not contain the namespaces.

  • import admin/utils/extract --> returns import utils/extract
  • import tags/common/utils/sortTags --> returns import tags/utils/sortTags

Technically extensions can also use the incorrect namespace (forum/admin and common) as this code doesn't check whether it's correct. This shouldn't be a problem that we focus on, however.

Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Seems to work when tested locally, makes sense to me! Do we want to add a deprecation warning for the old system? Or let these exist in parallel before deprecating the old one? Maybe next release we should sweep through and deprecate all the old systems (including the @flarum/namespace/path one)?

@dsevillamartin
Copy link
Member Author

Do we want to add a deprecation warning for the old system?

What would this deprecation warning look like? We'd only want to log one message because if we do every deprecated import, the console would be flooded with these messages.

Or let these exist in parallel before deprecating the old one?

Yeah I think we should deprecate these in the next release.

Maybe next release we should sweep through and deprecate all the old systems (including the @flarum/namespace/path one)?

Yes, though I think the @flarum/core is for public API so we wouldn't want to do that? Still not entirely sure.

@askvortsov1 askvortsov1 merged commit 67a2aac into master Dec 7, 2020
@askvortsov1 askvortsov1 deleted the ds/allow-compat-namespaces branch December 7, 2020 18:25
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.

4 participants