| justafish |
Along with a patch here to install it with composer |
| justafish |
I’d like to have this under the drupal namespace but @mixologic had some questions about it - fire away! :grinning: |
| webchick |
Wow, those set up instructions look dreamy! Trying them now! :smile: |
| justafish |
lol |
| justafish |
yeah the core patch will allow you to do `composer require` as the only step |
| justafish |
(also they’re assuming you don’t already have Drupal running somewhere) |
| mixologic |
Ah, yes. first thing im curious about is the eventual goal for where this is going to live on an ongoing basis... is this 'official' at this point, is it still experimental etc.. |
| justafish |
my preference is for it to stay on GitHub, but we can push it to a d.o remote as well as part of our build process if you like |
| justafish |
not sure what you mean by official vs experimental |
| mixologic |
Do you mean 'stay on github until its proven, then moved into the core repo' or do you mean, "from now on, we'll develop the core admin theme on github" |
| justafish |
preferably the latter |
| drpal |
+1 for keeping this on github. |
| mglaman |
Even when part of core? |
| drpal |
Yes. |
| mglaman |
(or is intent not to be part of shipped core) |
| mixologic |
Thats a dramatically bigger decision than Im comfortable supporting. |
| mixologic |
Not saying we dont end up there, just saying that I dont think we can just make that call without some technical working group process or something like that. |
| webchick |
We are also a long ways off from this being remotely production ready, or even ready as a thing to include in the core tarball as experimental, so I think we have time to have those discussions. |
| justafish |
yeah |
| justafish |
agreed |
| webchick |
The earliest this team is hoping to have something in drupal.tar.gz is 8.7, afaik |
| lauriii |
agreed with Angie, let’s figure out that part sometime later |
| justafish |
but is that blocking us using the `drupal/` namespace on packagist? |
| webchick |
In the meantime, they’re hoping to leverage GitHub’s status as “the” place JS developers come as a means of making rapid progress. |
| mglaman |
@mixologic is part of your concern that once on packagist, near impossible to put on drupal packagist |
| mixologic |
Sure, but staking something out in the drupal/namespace is permanent, not reversable/iterable.. so my suggestion is that we just use jsdrupal/ namespace, *or* we use the installers extention. |
| mglaman |
that's where I see benefit of `jsdrupal/***` |
| mixologic |
@mglaman well, no. its permanent once on packagist. |
| mixologic |
However, the composer script in the patch might not need the namespace at all. |
| mglaman |
Yeah, that's what I meant. If it's `drupal/admin-ui` then it forever has to be github / packagist |
| mglaman |
But, also.. if JS is where JS live and thrve I get the UX/DX reasoning |
| mixologic |
I think what we might be able to do to is add a new package type for composer .. call it "drupal-react-admin" or something like that , that installs it to the proper location |
| dawehner |
I played around with it, and well I'm not sure its worth the additional complexity |
| justafish |
agreed, it’s a massive pain |
| justafish |
the concern that was raised to me is that the core patch above is more likely to get in if it’s under the drupal/ namespace |
| justafish |
I’m fine keeping it with jsdrupal |
| dawehner |
@mixologic You say it is out there forever, but this is keyed by version number, right? We could add a new major version and noone would recieve this package anymore ... |
| mixologic |
That probably leads me to my second question. Is the goal to build 'one' react administrative interface, or is it to allow drupal to support 'react applications' ? |
| webchick |
I think both. |
| webchick |
By dogfooding our “one” react application, we will naturally uncover horrors that once fixed make it easier for anyone else to make a react application. |
| justafish |
indeed. I think that’s getting a bit off topic here though |
| mglaman |
So side task / other discussion on namespace hell :stuck_out_tongue: |
| justafish |
so the question is if the package is under jsdrupal is that a blocker for ? |
| webchick |
I do see @mixologic’s point though that this feels like a bigger decision than ought to be made in a JS channel on Slack with 7 people paying attention. |
| mixologic |
I think the topic I want to discuss, and maybe its in another thread, is how do we build react apps into core. |
| justafish |
@webchick @alexpott do you have any thoughts on the namespace? (as core committers :slightly_smiling_face: ) |
| dawehner |
@mixologic Please add this to the agenda :slightly_smiling_face: |
| mglaman |
I don't know why it would be, considering it's like any other dependency.
Honestly it'd be easier if we had `asset-pckagist` and set it as a `drupal-theme` |
| mglaman |
so the project lived on npm instead of composer. |
| webchick |
@justafish I honestly don’t understand what’s being asked. :slightly_smiling_face: |
| justafish |
@webchick in the patch here it’s pulling in a project from justafish/drupal-admin-ui |
| webchick |
Right. |
| justafish |
I was told that it being under my name would be a problem |
| justafish |
so if I move it to jsdrupal/drupal-admin-ui will that be a problem or is that fine? |
| webchick |
I don’t understand how we can commit that patch to core at all, unless this admin UI has undergone core code review. |
| webchick |
Like, is at least “alpha” quality |
| alexpott |
I think starting with jsdrupal is fine - it allows us to change it later. |
| justafish |
@webchick the patch doesn’t install the admin UI |
| webchick |
Because it would mean all people who downloaded Drupal would get this completely unvetted admin theme, no? |
| justafish |
no |
| mixologic |
This patch makes it easier for folks to be able to evaluate the admin ui. |
| webchick |
Ah, see, this is why I shouldn’t weigh in on these things. :smile: |
| justafish |
composer doesn’t have post install hooks |
| mixologic |
by making it easier to install. |
| alexpott |
@webchick that patch doesn't do anything unless you do composer require... |
| justafish |
so we have to monkey patch it into Drupal |
| webchick |
@alexpott so you are +1? |
| webchick |
Like, to me, this would be something in drupal-admin-ui’s composer.json, to require that patch. |
| webchick |
Vs. something in Drupal’s composer.json, to allow people to require something from some library that doesn’t even live in our repo |
| alexpott |
yeah it's okay - we should add docs to explain etc... but it's not API and many projects don't even use it. |
| webchick |
But I know composer.json isn’t the same as drush.make in this respect, so not sure what options there are for monkeypatching an upstream requirement |
| justafish |
@webchick we can’t do it in our own composer.json |
| webchick |
@alexpott cool. so i’d say as long as at least one other framework manager says +1, it’s probably fine. doesn’t really feel like a product decision. |
| webchick |
unless I’m missing some nuance. :slightly_smiling_face: |
| justafish |
@lauriii ^ |
| lauriii |
what do I have to sign on? jsdrupal namespace for at least during the development phase? |
| alexpott |
@lauriii just for now. We could change it during the development phase. It's just to make it easy to get working on the react now. |
| lauriii |
yeah, for me it’s totally fine for now |
| dawehner |
+1 for not relying on the DA to have to do any work |
| mixologic |
I think this is an expedient way to get to an easier evaluation of the project at hand, but that we'll still need a product level "how should drupal integrate with js apps" kind of decision down the road. |
| lauriii |
can’t really think of many downsides on this approach |
| mixologic |
and yeah, jsdrupal gives everybody here control over the packagist setup as well if there's anything that needs manipulating.. |
| justafish |
ok, I’ll update the patch. Thanks everyone :+1: |
| mixologic |
@dawehner I cant thread on a thread, but I'd like to chat with you about the composer installers you mentioned earlier.. |
| dawehner |
"how should drupal integrate with js apps" Well, that's fundamentally a problem this initiative tries to solve, so this won't get forgotten |
| mixologic |
is pretty relevant to that eventual discussion |
| justafish |
@mixologic @dawehner there’s an issue in GitHub if you want to discuss this further |
| mixologic |
great thanks. |
| drpal |
I guess some yml thing. |
| gabesullice |
Sorry for needing some backfill, why does Drupal need to know at all? |
| gabesullice |
Does Drupal just serve the react js code if the route is in the list? |
| lauriii |
The reason I wanted to discuss this was that this seemed something that is complicated and could potentially break easily. Have we considered the option of doing all routing on the backend (at least in the beginning)? |
| lauriii |
This could also enable us with server side rendering |
| justafish |
@gabesullice so for now we’re only building out a few known pages, so we need to export this to Drupal basically |
| dawehner |
I think this is not only complicated, its a fundamental design decision impacting every module |
| lauriii |
the architecture would be also simpler if we wouldn’t build a single page application |
| gabesullice |
@justafish, it's still not clear to me why Drupal needs to know. |
| dawehner |
we have a couple of needs |
| gabesullice |
If React takes over, won't it just not send a request to Drupal at all? |
| dawehner |
a) Clicking a link in Drupal should render react |
| gabesullice |
ah, so if an "overridden" route hits the Drupal router, then Drupal needs to know to serve nothing or just something to bootstrap React? |
| justafish |
@gabesullice because we’re falling back to Seven, so if you then click a link in Seven to /admin/people/permissions it needs to know to go to the React app and not load the internal route |
| justafish |
correct :+1: |
| lauriii |
there must be one application that is always aware of all routes |
| dawehner |
b) clicking a link in the react app might need to fallback to Drupal |
| gabesullice |
@justafish ++ |
| justafish |
@lauriii we did consider this, @drpal and @tedbow built a proof of concept |
| lauriii |
any blockers / downsides / reasons why it was dumped? |
| drpal |
@lauriii said poc, |
| justafish |
@lauriii we don’t want to couple the app to Drupal’s rendering system/other internals |
| lauriii |
@justafish we could do it in a way it can work as a standalone |
| tedbow |
@lauriii I guess I am still thinking about this. I really hadn’t seen the other idea till today so still wondering |
| tedbow |
I was wondering if we could use the same components that work as standalone or the way @drpal and I were doing |
| lauriii |
@justafish is the requirement from your side that the app must be able to function by itself (at least somehow)? |
| lauriii |
just trying to understand what is it that is required |
| justafish |
@lauriii one nice thing about this way is that you could potentially point this at any Drupal site, it doesn’t have to be your local one |
| dawehner |
I'm wondering why we not pick the decision and continue with it, till we run into fundamental problems |
| justafish |
(eventually) |
| alexpott |
For me we need to be careful not to create circular dependencies. The less aware the React app is of Drupal aware the better. And less it depends on too. |
| lauriii |
I do think that the current approach is complex, thus likely to cause unexpected issues |
| lauriii |
unless there’s some fundamental problems with the other approach, I suggest that we at least try it out |
| justafish |
@lauriii what do you find complex about it? It seems far more simple to me |
| justafish |
the other way we have to have a lot of support code within Drupal, we don’t have a build process for packaging it up, we’re mixing in 2 JavaScript/CSS systems |
| lauriii |
it seems a bit off for me that a client is telling backend about it’s business. I mean, it is someone that most of us would never have experienced with |
| lauriii |
however, routing on the serverside is something a lot of people have done |
| justafish |
Whatever approach we chose we have to have that for now |
| lauriii |
so as long as we are fine with the fact that our React application wouldn’t be a single page application, we could at least create PoC for this approach |
| justafish |
@lauriii I’m not sure what approach you’re advocating for exactly |
| justafish |
> so as long as we are fine with the fact that our React application wouldn’t be a single page application
fwiw I am not fine with this |
| lauriii |
approach where we do all the routing on the backend, and tell the client which route it is supposed to be rendering |
| justafish |
@lauriii we will do that eventually, we’ll need a way for Drupal to tell the app which component to render at which route |
| justafish |
and modules to provide their own components etc |
| lauriii |
why do we initially define the routes in the client, and not in the backend? |
| justafish |
@lauriii because then we can start working on pages in the admin app without you having to update Drupal |
| lauriii |
@dawehner what is the behaviour in the current approach if route exists in both, the backend and the client |
| justafish |
eventually that will go away and the app won’t define any routes and will get it all from Drupal |
| lauriii |
I see, so this is a temporary solution? |
| dawehner |
@lauriii we talked about annotating routes in Drupal to say: this is overridden by the app |
| justafish |
@lauriii yes |
| lauriii |
ummh |
| lauriii |
I didn’t take that into consideration before, sorry about that |
| justafish |
np! |
| dawehner |
@lauriii I agree that this is a tough problem, and is fundamental, so let's not forget about it |
| justafish |
@lauriii are you happier with the approach now? |
| lauriii |
I’m trying to think what potential downsides there would be in having this as a temporary solution |
| lauriii |
so is the plan not to have server side rendering for now? |
| justafish |
correct |
| justafish |
(or maybe ever) |
| dawehner |
Given we would need node as requirement this is a really far streched future |
| justafish |
yeah |
| drpal |
or phpv8js |
| lauriii |
yeah, I don’t have experience working with that but it seems lucrative |
| lauriii |
but I’m not sure how useful it would be. I have no idea how configurable it is, like let’s say I’m not sure if it’s possible to implement fetching dependencies on the backend using that |
| lauriii |
is the original discussion visible somewhere? |
| drpal |
@lauriii around? |
| dawehner |
@drpal that's a bigger requirement |
| lauriii |
around the routing |
| justafish |
@lauriii no I was on a hangout with them earlier today and we got onto the subject |
| justafish |
so, here is the discussion :laughing: |
| justafish |
I don’t think server rendering would be very useful for the admin theme fwiw |
| justafish |
considering the effort and requirements that we’d need to do it |
| justafish |
and maintain it |
| lauriii |
we might have to provide nojs fallback on some pages such as content authoring pages anyway |
| justafish |
how come? |
| lauriii |
those pages are not only used by admins |
| lauriii |
but some users use them as a functionality on their sites |
| lauriii |
anonymous users can do content authoring in some cases |
| justafish |
they can always enable a “classic” theme then :slightly_smiling_face: |
| lauriii |
are we going to support classic theme forever? |
| lauriii |
not to say this is necessary relevant right now |
| webchick |
I’m getting kind of a “spidey sense” from this conversation that we really ought to write up a formal “plan” issue in the ideas queue to ensure we have sign-off on all of the things that we’re planning to do. |
| webchick |
Using GitHub, using the jsdrupal namespace, architecting it the way we are planning, not providing non-js fallback, etc. |
| webchick |
these are all pretty ‘radical’ ideas, and there are good reasons for all of them, but i’m concerned that if there’s not visibility to esp. the core committers, we run a huge risk of doing a bunch of work that isn’t accepted. |
| webchick |
I’m happy to draft this, but will need some guidance on the finer points, esp. the architecture where things just turn into “blargy flargen glurg” for the most part. :slightly_smiling_face: |
| lauriii |
I support the idea of moving some of the big decisions to the issue queue |
| webchick |
And I think we can continnue to work as we’re doing as those discussions play out, but IMO fleshing out any “deal breakers” very early in the process is a good thing.
But. I also dont’ want to step on any toes. @justafish/ @drpal thoughts? |
| justafish |
go for it :+1: |
| webchick |
Cool. :slightly_smiling_face: |
| tedbow |
are there any issues at all or other docs that I can look at catch up? |
| justafish |
@tedbow I think the roadmap and Trello board |
| justafish |
Trello board link is in the room topic |
| tedbow |
@justafish thanks |
| dawehner |
Given the previous remark, maybe we should really focus on the site builder user |
| webchick |
I don’t think we need +1s, really. We need a framework manager to say “LGTM” and we need an RTBC. |
| webchick |
Sign-off from product management’s going to be trivial. |
| justafish |
wassup @alexpott! :laughing: |
| alexpott |
dunno... busy collecting titanium |
| dawehner |
I think one thing to keep in mind though: We aren't locked as in, we can mock the endpoints |
| webchick |
Yeah, and I’m curious how helpful, at all, JSON API will be if we’re doing permissions page as our PoC. |
| webchick |
No content entities of any kind. :slightly_smiling_face: |
| justafish |
config entities |
| tedbow |
the permissions are plugins |
| webchick |
Right, but does JSON API handle either of those? |
| webchick |
I thought that’s why @gabesullice was pushing last week for us to choose admin/people or similar as our target use case vs. the perms page |
| gabesullice |
JSON API cannot handle anything for that permissions page except which permissions are enabled for each role |
| gabesullice |
(those are config entities) |
| webchick |
And read-only on that, correct? Or can we do full CRUD on those? |
| gabesullice |
but an actual listing of permissions (like machine name, human name and whether there should be a sec. warning) cannot be handled by JSON API |
| gabesullice |
read-only |
| gabesullice |
I think last week and up to this point there has been discussion of making a custom REST resource for this |
| webchick |
Correct. Hence my confusion of this group promoting JSON API. |
| webchick |
But not a bad thing. :slightly_smiling_face: |
| gabesullice |
but there has also been some discussion between myself, @dawehner, @e0ipso and @wimleers about coming up w/ a more holistic approach |
| webchick |
Yeah? |
| gabesullice |
Yeah, we've discussed following something like JSON-RPC protocal and adding a new annotation/plugin manager for RPC calls that could do things like list/enable permissions or clear cache for example |
| gabesullice |
but it's really just an apple in my eye at this point |
| gabesullice |
no one is super opposed, but still early |
| webchick |
Interesting! |
| gabesullice |
|
| gabesullice |
the advantage being that we could autogenerate docs and an JS API client |
| gabesullice |
while custom REST resources for everything the admin theme will need will not be able to do anything automatically |
| justafish |
> JSON API cannot handle anything for that permissions page except which permissions are enabled for each role |
| justafish |
we need this :slightly_smiling_face: |
| justafish |
I know we’re not going to be able to write to it for now |
| e0ipso |
@webchick the +1's were more in the vein of showing consensus or opposition from the community. It's been a while since the comments on that issue were posted and I wanted to make sure positions had not drifted since then. |
| dawehner |
I think the best approach would be to provide some implementation for the JS initiative and work on something more mature in core |
| webchick |
@e0ipso fair enough. :slightly_smiling_face: |
Comments
Comment #13
justafishComment #14
justafishComment #16
justafish