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

Bring back common, forum, and admin namespaces in JS #2085

Open
datitisev opened this issue Mar 25, 2020 · 4 comments
Open

Bring back common, forum, and admin namespaces in JS #2085

datitisev opened this issue Mar 25, 2020 · 4 comments
Labels

Comments

@datitisev
Copy link
Member

@datitisev datitisev commented Mar 25, 2020

Related to #1821.

I think it'd be in our best interest to bring back the common, forum and admin namespaces that were removed at some point (I think beta 8).

Pros:

  • we don't have to worry about naming files the same in common and forum/admin causing issues (because of flarum.core.compat)
    • also prevents issues when we generate typings - using flarum/forum/Forum instead of flarum/Forum
      img
  • makes knowing where files are located in core easy when looking at extensions (no need to guess namespace)
  • if looking at core, can easily see what the import should be (no need to remove part of it)
  • easier to understand from the perspective of a newly starting extension developer (luceos, #2085 (comment))

Cons:

  • definitely means ALL JS exts will stop working
  • @clarkwinkelmann thinks it'd be more readable, likes bundling & common

What we could do is allow both ways- keep the current structure without the stack in flarum.core.compat BUT also allow including the stack when typing code (stripped on build). This could cause confusion, however.

If I missed any pro/con, please comment so.

@luceos

This comment has been minimized.

Copy link
Member

@luceos luceos commented Mar 26, 2020

It's also easier to understand from the perspective of a newly starting extension developer.

@tankerkiller125

This comment has been minimized.

Copy link
Member

@tankerkiller125 tankerkiller125 commented Mar 26, 2020

I feel like the majority of JS extensions will probably break in the rewrite anyways. I think the benefits far outweigh the cons here. Just making it easier to understand for new extension developers I think alone outweighs the cons.

@clarkwinkelmann

This comment has been minimized.

Copy link
Contributor

@clarkwinkelmann clarkwinkelmann commented Mar 26, 2020

We currently accept using the /admin and /forum imports when using @ imports https://github.com/flarum/flarum-webpack-config/blob/master/index.js#L55

We can add that as well as /common support to the "old-style" section so that developers can prepare their extensions for the upcoming change.

@datitisev

This comment has been minimized.

Copy link
Member Author

@datitisev datitisev commented Mar 26, 2020

@clarkwinkelmann I don't think there's a single extension that uses @flarum/core imports - mainly because core exports barely anything and it's not documented in the slightest.

I didn't even know we had JS extenders 🤷‍♂

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.