🇺🇸United States @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.

Marking as fixed since this was merged in.

@AaronMcHale there is actually an issue and a related PR in the Starshot prototype repo:

https://github.com/phenaproxima/starshot-prototype/issues/17
https://github.com/phenaproxima/starshot-prototype/pull/61

@cosmicdreams are you talking about a regular tagged release here, or a major version / breaking change?

If you enable through the UI it will give a warning about enabling experimental modules. And generally I’d imagine it isn’t uncommon for projects to not allow experimental modules. It also can’t be disabled because it is an explicit dependency.

Neither situation is perfect, but I still think we’re better off removing the dependency.

Created the following issue in the LWG project as a next step: https://www.drupal.org/project/drupal_lwg/issues/3447461 →

tekNorah → credited brianperry → .

For additional context, here is a somewhat cross copy and paste of the Slack discussion mentioned above.

brianperry
10:51 AM
Hi licensing folks. For the Drupal API Client we initially added a GPL License to the project, but have received feedback that this will likely be problematic given the fact that most JavaScript projects that this will be used with use the MIT license. We'd like to change to MIT but wanted to check in here to see if there were any complications due to the fact that we're being hosted on Drupal Gitlab.

brianperry
10:58 AM
This section of the licensing page seems to imply that all Drupal contributed projects should use GPL. Wondering if that interpretation is correct, and if there is an opportunity for exceptions for general / JS projects that will be distributed via NPM and will in most cases be used outside of a Drupal codebase.

hestenet (he/him)
2:34 PM
This may have to be a discussion of an extension of question 4 - https://www.drupal.org/about/licensing#link-gpl-incompatible →
Drupal.org
Licensing
These are answers to frequently asked questions about licensing of the Drupal project.
Nov 11th, 2015 (39 kB)
https://www.drupal.org/about/licensing#link-gpl-incompatible →

2:35
The last time we updated the FAQ was before we were hosting Drupal-agnostic JS components..

effulgentsia
10:57 PM
I think it's more about question #2. IANAL but sounds to me based on the wording of that, that if someone downloads the code from drupal.org, then it's GPL. If the original code is licensed MIT and someone wants to get that code with its original MIT license, then they have to get it from somewhere else. So hosting the repo on GitHub and mirroring to Drupal GitLab could be an option? Or, drupal.org could change its policy and that FAQ accordingly.

hestenet (he/him)
12:07 AM
You could come at it from either angle, sure. Drupal interprets the GPL to mean that certain types of non-GPL assets can be included with Drupal 'in aggregate', vyt we have not historically been the source of hosting such assets, and so our policy hasn't specifically spoken to that case.
The assumption of #2 has implicitly been that code hosted by d.o was going to be Drupal code, not other independent libraries, so it was written expecting to enforce that Drupal derivative code must fall into GPL. That may not reflect the full reality of what we want to do with d.o today. (edited)
12:09
I'd generally be in favor of updating the policy to clarify the GPL requirement for Drupal-derivative code, and the allowance of GPL-compatible licenses for non-derivative code that could be included 'in aggregate' (i.e - via dependency management process) (edited)

brianperry
4:10 PM
The assumption of #2 has implicitly been that code hosted by d.o was going to be Drupal code, not other independent libraries, so it was written expecting to enforce that Drupal derivative code must fall into GPL. That may not reflect the full reality of what we want to do with d.o today
Biased here, but I think the existence of 'general' projects breaks that assumption. This for example is a library that interfaces with Drupal endpoints, but in most cases won't actually be used in a Drupal codebase. If either the fact that it exists on drupal.org, or talks to Drupal means that it has to inherit GPL I think that could limit the utility of general projects.

brianperry
4:16 PM
I'd generally be in favor of updating the policy to clarify the GPL requirement for Drupal-derivative code, and the allowance of GPL-compatible licenses for non-derivative code that could be included 'in aggregate' (i.e - via dependency management process)
Not sure if 'GPL-compatible' was a typo above. My ideal here would be a GPL requirement for derivative code, and allowing GPL incompatible licenses (MIT in this case) for non-derivative code.

brianperry
4:21 PM
Ah, the 'GPL-compatible' link seems to imply that the various licenses that fall under the distinction of MIT would be considered GPL compatible, so that makes sense to me.

hestenet (he/him)
7:11 PM
Yup. I think we're more or less on the same page.

brianperry
12:52 PM
@hestenet (he/him)
given that, what would be the best next step here? Creating a related issue in https://www.drupal.org/project/drupal_lwg → ?
New

hestenet (he/him)
1:00 PM
Yes please!
1:00
Mention my name and maybe even copy this convo.

hestenet (he/him)
2:50 PM
@brianperry
Did that issue get created?

brianperry
3:49 PM
@hestenet (he/him)
Didn't get a chance, but I'll create one no

brianperry → created an issue.

> Original issue meaning the large buttons?

Yup the large buttons which is a symptom of the assets for our single file components not loading.

> Add defensive programming tatics that detects if object is null and attempt secondary object retrieval logic if needed.

I don't think this was the root cause, but I pushed up a commit that should protect from this as well using optional chaining.

I believe I have a fix for this ready. Safari was choking on our uses of dispatchEvent. I'm not sure why - Safari supports dispatchEvent. Changing to click() made Safari happy again.

Thanks @rkoller for the help debugging here. Super helpful.

brianperry → created an issue.

This (rightfully) came up in the Starshot thread about SPP. Assigning it to remind myself to take another look.

Re-opening.

When enabled as part of an initial install this is fine (which unblocks this for the Starshot prototype) but I'm still seeing the original issue when later installing this in the UI or via drush. Probably not as high a priority, but still an odd install thing that we should probably get to the bottom of.

On the current 10.4.0-dev if I install Drupal, and then install this module I get the error that @pcambra mentioned. I'm only able to use this module if I enable the experimental SDC module.

So I don't think we'd want to add the experimental SDC module as a dependency - my understanding is that going forward components / SDCs are just a fundamental part of Drupal that don't need to be enabled. Instead, we need to figure out what this sdc_display is relying on from the experimental module and fix that.

Could see either side on this one, but I'd rather not restrict the audience for this module again. It does add a potential hiccup for pre 10.3 users, but I think the benefit of a wider potential audience outweighs the risk.

brianperry → created an issue.

Discussion on this topic in Drupal #licensing slack: https://drupal.slack.com/archives/C204BR9ML/p1714924304161589

Need to figure out concrete next steps, but it sounds like we might have a path forward here.

Thanks to all who attended. Great conversation!

I've done some experimentation here and think this is achievable. The possible solution is quite different from what I expected though.

I came to the same conclusion that using json-schema isn't currently viable. I'm light on some of the details here because I did some of this a few weeks back and then had to set it aside, but there were two main issues.

* Using the jsonapi_schema module I found some of the results for resources unstable. I unfortunately can't remember the specifics, but some key resources we'd need were returning 500s. Will try to reproduce this at some point and create an issue in the jsonapi_schema project.
* Even if that was stable, I found that packages like https://www.npmjs.com/package/json-schema-to-zod would choke on schemas with relationships due to circular references. It kind of makes sense given the JSON:API spec, but it is extremely problematic for this use case.

So based on that, I don't see a path forward with json-schema.

On the other hand, I found that json-to-zod works really well.

I created a small POC here: https://github.com/backlineint/schema-gen-poc

It actually just uses json-to-zod with the standard JSON:API endpoints to generate then schemas we need. Even more amusingly it actually uses our client to feed json into json-to-zod. Here is the entire POC script:

import * as fs from "fs";
import { JsonApiClient } from "@drupal-api-client/json-api-client";
import { jsonToZod } from "json-to-zod";

const client = new JsonApiClient(
  "https://dev-drupal-api-client-poc.pantheonsite.io"
);
const zodImport = "import { z } from 'astro:content';\n\nexport ";

const writeSchema = (schema, fileName) => {
  fs.writeFile(fileName, zodImport, { flag: "w" }, (err) => {
    if (err) {
      console.error(err);
    } else {
      fs.appendFile(fileName, schema, (err) => {
        if (err) {
          console.error(err);
        } else {
          // file written successfully
        }
      });
    }
  });
};

console.log("Generating schemas...");

// Generate a schema for an article collection.
const articles = await client.getCollection("node--article", {
  queryString: "include=field_media_image.field_media_image",
});
const articlesSchema = jsonToZod(articles).replace(
  "const schema",
  "const articlesSchema"
);
writeSchema(articlesSchema, "src/lib/schemas/articlesSchema.ts");

// Generate a schema for a media--image resource
const mediaImage = await client.getCollection("media--image");
const mediaImageSchema = jsonToZod(mediaImage.data[0]).replace(
  "const schema",
  "const mediaImageSchema"
);
writeSchema(mediaImageSchema, "src/lib/schemas/mediaImageSchema.ts");

// Generate a schema for a file resource
const file = await client.getCollection("file--file");
const fileSchema = jsonToZod(file.data[0]).replace(
  "const schema",
  "const fileSchema"
);
writeSchema(fileSchema, "src/lib/schemas/fileSchema.ts");

The intent is to run that in advance and commit the schemas to source.

The experience of using those Zod schemas in code is really nice. This POC renders a list of articles at /blog. When sourcing article data, you'd do this:

import { articlesSchema } from "../../lib/schemas/articlesSchema";

type Articles = z.infer<typeof articlesSchema>;

const articles = articlesSchema.parse(await client.getCollection<Articles>("node--article", {
  queryString: "include=field_media_image.field_media_image",
}));

Zod will throw an error when parsing if something violates the schema. You also get full intellisense/autosuggest when working with articles on the template.

So I think this could work. What I don't know at the moment is how we could make this into a more generic utility for users. Open to any ideas. Or feedback on this approach in general.

brianperry → created an issue.

Main remaining thing I think we need to do here is add Next.js Pages Router examples. Then we can close this and split out any other lingering things into standalone docs issues.

brianperry → created an issue.

Closing this for now. I think our current workflow is reasonable without this feature.

We've prioritized https://www.drupal.org/project/api_client/issues/3404988 → which would provide a way to invalidate or refresh the cache. We should solve that first, and then that will likely make our options here clearer. Leaving this as postponed for now.

Closing this. From my perspective we currently support:

* Basic Auth
* Simple Oauth
* Cookie Auth in a partially decoupled Drupal app (just works, didn't require any customization)
* Custom auth headers.

We also have an issue for JWT auth: https://www.drupal.org/project/api_client/issues/3376949 →

Also open to supporting other methods, but would like it to be driven by demand.

Want to highlight this from @rlnorthcutt's thoughtful post.

> Drupal is incredibly good at letting us generate HTML, and the templating system means we can generate it down to the field level. So, with very minor changes, we should be able to allow Drupal core to easily return HTML fragments of any entity template. Add in display modes and views, and suddenly you have the ability to create an entire HTMX backend with little to no code. Thats insanely powerful, and can give the project a shot in the arm. Instead of building a custom backend in python, laravel, or go... just use Drupal.

Had some similar comments in the HTMX issue queue 🌱 1.1 Goals for HTMX Active . A relatively small amount of functionality could make Drupal the ideal HTMX backend.

Also worth noting, Astro recently released Page Partials as a feature targeted at this use case.

A question on slack about how this compares to the Next for Drupal client: https://drupal.slack.com/archives/C1AKSFBEW/p1714540629657629?thread_ts=...

Might be able to surface this in the docs better.

@fago helpfully raised this issue once again on Twitter.

Having GPL currently is a combination of the source being on Drupal gitlab (I read that as being implied here: https://drupal.org/about/licensing#content-license) and also needing to have a license in the repo early on when establishing an Open Collective.

I'd be open to changing this if possible, and agree that it would be a good discussion for the community. MIT was proposed as an alternative, which I personally prefer. It is also the common standard in the projects that we have modeled this work on.

I'll look at trying to start discussion with the licensing working group. Might also be able to catch someone face to face in Portland if necessary.

Was also working on getting HMR working with a React app, and ran into the same issue. The functionality in the MR work well for me. @tanc the only thing I'd suggest adding is some documentation in the readme about the new devDependencies option so that people are aware of it.

Opened an MR with the proposed fix.

Thanks for this extremely useful module!

brianperry → created an issue.

Marking as fixed as we've completed our commitment.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.

brianperry → created an issue.