🇺🇸United States @brianperry

nicxvan → credited brianperry → .

Would creating a generic `islands` SDC make sense here?

brianperry → created an issue.

teknorah → credited brianperry → .

> I think npm workspaces might be what you're after it lets you built multiple packages in a single monorepo.

I talked myself out of adding this comment previously because I know core is standardized on Yarn, but pnpm is extremely good at this as well. https://pnpm.io/workspaces

I'd also add a -1 against Lerna. All I ever hear about Lerna these days is people desperately trying to migrate away from it.

> It looks like the Api Client module has been created in the meantime (by @brianperry, et al ;-).

This is true! Welcome any feedback as these packages hopefully gain adoption. There is also a follow up issue related to promoting the packages to the Drupal namespace 🌱 Promote Drupal API Client Packages to Drupal NPM Namespace Active if adoption warrants it in the future.

Published: https://www.npmjs.com/package/@drupal-api-client/graphql-client

brianperry → created an issue.

brianperry → created an issue.

Thanks for sharing here @balintbrews. Following along with this issue and other XB work has pretty directly led to experiments like this, so happy to share anything I can.

> @brianperry: what are your thoughts on pros/cons between 11ty islands vs. Astro islands?

@effulgentsia the main reason I started with 11ty's implementation is that Astro didn't seem to have an easy way to use the astro-island element outside of a full astro project (unless I'm missing something). 11ty's implementation is focused on exactly that. The readme calls it "a framework independent partial hydration islands architecture implementation" and it doesn't require 11ty to use.

It's a lot simpler than I expected once I dug into it. Most of the code is the the expected client loading directives, and then it adds some simple utilities for module imports, automatically initializing various frameworks, and replacing fallback content.

Being framework independent also means that pretty much everything the package does is on the client. So it won't help with any of the things the MR on this issue does around compiling. It can work with 11ty's server rendering features, but it could work SSR from other frameworks, or as this module is trying to prove - content server rendered by Drupal.

The other noteworthy difference is that @11ty/is-land doesn't seem to have built in support for React. I don't know if this is directly related to the challenges of using JSX without a compile step, or just that the maintainer isn't a big fan of React based frameworks.

So for what this issue is setting out to do, my gut would be that you're still better off with Astro. The Islands module currently isn't much beyond the provider of a re-named is-land element, along with a naming convention based approach to import maps that will become obsolete once Drupal has formal support. I have issues in the queue to go deeper into dealing with Drupal content, and also things that will require some kind of compilation step - if I come up with anything interesting I'll be sure to share it here and/or in Slack.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

johnpicozzi → credited brianperry → .

johnpicozzi → credited brianperry → .

> The importmap functionality is really only when you have unknown consumers. E.g. React module doesn't know what themes/modules will need to load react.

I get that this is the primary use case driving this issue, but I don't know that I agree with this statement broadly. Import maps also simplify the process of using esmodule imports in unbundled code. It would be possible for example to create a small utility in a vanilla js file that is exported as a module, which could then be imported in other js module files where needed. You don't necessarily need import maps to do that, but Drupal's library system / asset handling can make it tricky to negotiate module paths without something like an import map.

In practice most complex projects will use a bundler, but this pure esm approach seems like something that could grow over time.

Symfony's Asset Mapper component uses import maps extensively. In fact, they seem to recommend asset mapper as a default

Elsewhere in the FE ecosystem the Islands Architecture popularized by Astro is built on top of esm imports. Their `client:*` directives result in a custom element that loads the necessary javascript via a dynamic import.

While this issue sparked questions related to SDCs since I have been working with them lately, I'm also following this because I've been experimenting with a Drupal Islands Architecture implementation. Based on experiments thus far, I think a Drupal import map implementation would be needed to make this viable. The current POC builds its own import map. It isn't really packaged up for easy consumption at the moment, but I'll try to follow up here when that changes.

> @brianperry can you elaborate a little more on the use case (In slack if not here)? I hit some things that might be relevant, but I'm not totally sure.

@mortona2k I'll say hi in the slack channel, but I'm mostly thinking about the un-bundled use case above.

Really excited about this feature!

Would probably outside of the scope of this issue if there is any follow up, but after reviewing the in-progress implementation it got me thinking about how this would apply to single directory components.

My best guess is that you'd currently have to:

* In the component.yml for the SDC itself, override the library to add the module attribute for the js asset:

  js:
    my-component.js: { attributes: { module: true } }

* Add an importmap.yml file at the root of the module or theme that defines the single directory component. In that file you'd need to reference the library by the name auto-generated by sdc (core/components.[THEME_OR_MODULE_NAME]--[COMPONENT_NAME_WITH_DASHES])

That would at least make adding js from a component to the import map possible on the current feature branch, which is great. But it feels like there could be some syntactic sugar in the sdc module to make this easier. One of the great things (imho) about SDCs is that a developer can write a js file and not worry about defining a library. Would love to see that concept extend to import maps.

It seems like the `hook_importmap_alter` added here would make it pretty straightforward to implement this in the sdc module once it is determined what could be used to communicate that js should be included in an import map / handled as a module.

Thoughts?

> I don't think that Experience Builder is currently in violation of current d.o. policies around this. We are using MIT licensed code (e.g., React), but that doesn't prevent the Experience Builder module as a whole being licensed under GPL.

Admittedly out of my depth on this licensing stuff here, so apologies if I mischaracterized anything...

Cross posting a related thread from the Starshot Demo Design System slack: https://drupal.slack.com/archives/C07CDLSD8UU/p1727204460838479

Perhaps that design system could be an opportunity to revisit and move this issue forward.

Now that I think of it, Experience Builder itself will likely force our hand on some of this. I'd imagine the current work in progress already depends on quite a bit of code licensed under MIT.

@bnjmnm mentioned this Astro work in passing at DrupalCon (I was also going on about Astro,) but I wasn't sure exactly where it was bubbling up in XB work until I came across this issue. Super interesting stuff - will be following.

Have you seen Astro's experimental container API? It provides a way to render Astro components in isolation. Based on peeking at the MR I don't think it would really be needed here, but maybe it will spark some other ideas.

There is an Astro in PHP example out there, but I believe it requires node to be running on the same server. Of course, this MR provides access to web containers, so technically that gives us node. I've always wondered if some variation on this would make server rendering Astro components in Drupal possible...

Reviewed and commented on the in progress docs MR.

brianperry → created an issue.

brianperry → created an issue.

Merged and released as 1.2.0

New API demo site is live at https://drupal-api-demo.party/ :)

Updated the docs to use this for examples as well.

Removed outdated note about this being a contributed module.

As a first step, I pushed up a branch that converts the local environment to use recipes, and also committed the Drupal project to a separate repo. I'm hoping that makes it easier to both deploy to hosting, and ensure that the local dev environment matches the hosted dev environment.

Next I'll deploy this live somewhere.

brianperry → created an issue.

brianperry → created an issue.

Additional discussion in slack: https://drupal.slack.com/archives/C05BP6659U0/p1723227683199179

which spawned a few issues:
https://www.drupal.org/project/api_client/issues/3467667 →
https://www.drupal.org/project/api_client/issues/3467668 →

brianperry → created an issue.

brianperry → created an issue.

> Is there any way to achieve this in the meantime? I’m interested in making authenticated requests to a Drupal site to non-JSON API endpoints with various methods.

This issue was written with the json-api-client in mind, so how practical this is might depend a bit on what type of non JSON:API endpoints you mean here. Would love to hear some more details about your use case. But we did structure this project in such a way to make it possible to support more than just JSON:API.

If routing is really what you need and decoupled-router makes sense in your case, our decoupled-router-client is a standalone package: NPM / Docs

The base API client is also distributed as a package. You can extend this class to get things like our authentication methods, caching, and so on. So if what you're really looking for is a jump start to work with a completely different API type, this could help. NPM / Docs

Some other possibilities that don't yet exist, but have been discussed:

Super early, but we've started work on a GraphQL client. This will support mutations → which would address this use case.

There has also been discussion about the idea of creating a client for JSON-RPC → .

So yeah, a number of possibilities, but would love to hear more about what you are looking to do.

brianperry → created an issue.

brianperry → created an issue.

Created a draft MR with the implementation here. It doesn't include tests, as @d34dman was interested in contributing tests. This should provide a good testing overview as it requires tests for the new getView method, but also requires smaller test updates for full coverage for createUrl and createCacheKey.

brianperry → created an issue.

Ran into this issue and gave the existing feature branch a try. Doesn't seem to be working for me quite yet. Didn't have time to fully debug, but did push up a new branch with:

* The latest module code merged in.
* Changing jsonapi_hypermedia from a dev dependency to a suggested dependency.
* Adjusted the format of the link key to remove what I consider to be a redundant use of 'jsonapi'

Hope to get back to this one at some point.

brianperry → made their first commit to this issue’s fork.

nod_ → credited brianperry → .

nicxvan → credited brianperry → .

Got all the CI jobs passing and merged.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

Latest MR does in fact fix it. Awesome! Deployed the updated docs and everything is working as expected: https://project.pages.drupalcode.org/api_client/api/drupal-api-client/ap...

Based on the anticipated changes outlined in this issue, I've published a release of the Drupal API Client packages under the MIT license: https://www.drupal.org/project/api_client/issues/3367042#comment-15639252 →

Marking as fixed. Based on the general agreement with the LWG I've published a release of our packages that uses an MIT license: https://www.drupal.org/project/api_client/issues/3389581 →

Sorry I wasn't able to get to this myself, but the readme updates look good to me @tanc

I opened a draft MR that installs project dependencies via yarn at the start of the eslint job. I didn't test this. If someone could provide guidance on how I could test this branch with a contrib project, I'd be happy to do that with same_page_preview.

Short term, we should probably persist node_modules between jobs to speed up the yarn install. I'll leave that one to someone more experienced with the inner working of these CI templates (I assume something similar is already happening for composer.)

Longer term, we could:
* Detect if a package.json exists rather than blindly running yarn install.
* Use the package manager configured for the project (npm, pnpm, etc) rather than always using yarn.

But my guess is that this one line change alone could resolve many of the problems reported in this issue.

I think there are a couple of different related problems here. The main one is that while the eslint job will install node modules defined in core's package.json (this happens using yarn in the composer-base job I believe,) it doesn't install node dependencies from the module project's package.json. So given this .eslintrc

{
  "extends": [
    "drupal"
  ]
}

You'll always end up with "ESLint couldn't find the config "drupal" to extend from". The Drupal eslint docs → mention this in a later paragraph, but extending 'drupal' requires first installing https://www.npmjs.com/package/eslint-config-drupal. Core doesn't have this dependency, so the contrib project will have installed it. But project node dependencies aren't installed by the Gitlab eslint job currently.

So why are people seeing "ESLint couldn't find the config "airbnb-base" to extend from"? Core does have this as a dependency. It is essentially the same problem. From https://eslint.org/docs/v8.x/use/configure/configuration-files#extending...

Relative paths and shareable config names in an extends property are resolved from the location of the config file where they appear.

In this case, that is the contrib project directory, which won't have this dependency installed. And this is even considering that the CI job uses '--resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core' Unfortunately the extends property ignores that flag when resolving.

So without installing additional dependencies, you need to explicitly reference the plugin in Core's dependences (similar to how the CI job runs eslint from core's node_modules/bin) This .eslintrc should work:

{
  "extends": [
    "../../../core/node_modules/eslint-config-airbnb-base/index.js"
  ]
}

But projects that rely on dependencies that aren't in core will be out of luck. I think the only way to resolve that would be to install project node dependencies during the eslint job. I'll try to take a quick look to see if I can open an MR with initial changes for that.

Thanks for summoning me :) Will try to take a look at this and see if I can provide any suggestions.

Bumping up the priority on this one. Worth doing on it's own, but I also think it might be the only way to completely resolve https://www.drupal.org/project/same_page_preview/issues/3446177 🐛 CSS, JS not loaded on module install in Drupal 10.4.0-dev Needs work

griffynh → credited brianperry → .

> I didn't see a place in the docs to update this besides for the generated API docs which should be covered here. Let me know if a new section should be added or if this makes sense in the performance section let me know.

I think this is fine for now. In the future it might belong in an 'advanced options' section of the docs.

Added a changeset and merged this one. Thanks @coby.sher!

brianperry → created an issue.

This would be fun to solve, but postponing for now to focus on tightening up existing features.

Would still love to do this, but marking this as postponed for now as I think we focus on improving the module for 'traditional' Drupal.

Marking this as wont fix. We require ^10.1 currently and Drupal 9 is now EOL :)

Marking as fixed since we did create the issue on the ideas queue.

https://www.drupal.org/project/ideas/issues/3367356 ✨ Consider adding Same Page Preview to core Active

We did propose this in the ideas queue: https://www.drupal.org/project/ideas/issues/3367356 ✨ Consider adding Same Page Preview to core Active and there has also been discussion about inclusion in Starshot. Given all that, I'm not sure core inclusion is the real focus at the moment. More about trying to improve this module as much as we can, and following the Starshot threads.

Marking as postponed as a result.

Thanks for checking in on this @ruturaj-chaubey!

That back end code makes sense at a glance, but my assumption was that we could use https://www.drupal.org/project/jwt → for essentially the same thing.

I tried an example that issues a token in node.js that should be compatible with what the JWT module is expecting (uses the same key, has the same payload, etc), but if I try to authorize using the token I sign in node, it doesn't seem to work.

import jwt from "jsonwebtoken";

const token = jwt.sign(
  {
    drupal: {
      uid: "1",
    },
  },
  privateKey,
  { algorithm: "HS512", expiresIn: 60 * 60 },
);

const headers = new Headers();
headers.set("Authorization", `Bearer ${token}`);

fetch("https://drupal-client.ddev.site/jsonapi/action/action", { headers })
  .then((response) => response.json())
  .then((data) => console.log(data))
  .catch((error) => console.error("Error:", error));

My best guess based on inspecting tokens using the debugger at https://jwt.io/ is that it is a problem related to base64 encoding. I'll keep poking at this, but wondering if I'm missing something obvious, or if the JWT module can't handle this use case.

The current client can authenticate using Drupal issued JWT with something like this:

  // Get a JWT token using basic auth
  const headers = new Headers();
  const encodedCredentials = stringToBase64(`admin:admin`);
  headers.set("Authorization", `Basic ${encodedCredentials}`);

  const tokenRequest = await fetch(
    "https://drupal-client.ddev.site/jwt/token",
    { headers },
  ).then((response) => response.json());

  // Use the JWT with custom auth
  const jsonApiClient = new JsonApiClient(baseUrl, {
    debug: true,
    authentication: {
      type: "Custom",
      credentials: {
        value: `Bearer ${tokenRequest.token}`,
      },
    },
  });
  const actions = await jsonApiClient.getCollection("action--action");

It is also possible to refresh your JWT using the existing JWT:

  // Refresh your token using the existing token
  const jwtHeaders = new Headers();
  jwtHeaders.set("Authorization", `Bearer ${tokenRequest.token}`);

  const jwtTokenRequest = await fetch(
    "https://drupal-client.ddev.site/jwt/token",
    { headers: jwtHeaders },
  ).then((response) => response.json());

Next I'm going to try to work up an example where the token is issued by the JS app, but honored by Drupal.

This has been helpful though - I think what we need to implement here is the overlap between these two flows. Basically the client assumes that you have a valid JWT to start, and then uses (and updates) it from there. We can document how you'd get the initial token, or maybe create small helpers.

Finally got the eslint job passing here: https://git.drupalcode.org/project/same_page_preview/-/jobs/1647875

That code is part of the Gitlab CI MR here: https://git.drupalcode.org/project/same_page_preview/-/merge_requests/64 I'm hesitant to extract that into a standalone MR over here as it will result in a variety of future merge conflicts. I'd rather just get the remaining jobs passing and get that one through.

I'm going to try to write a blog post about what I did to get this passing - feels like something that will trip others up.

This would be helpful for contrib projects using Gitlab CI that want to make sure they can run equivalent linting locally prior to the CI job. I currently need to copy or extend Core's prettier config to make this happen.