Stream wrapper reference in JS library causes error in _locale_parse_js_file()

Interesting! I don't think it was ever designed to support that. Did that work in D7?

vuejs contrib affected as well

I need this for dynamically generated libraries, whose files should be able for being aggregated by core. Once #1308152: Add stream wrappers to access .json files in extensions is in, would this mean we'd then still need to write a fix for the mentioned bug here?

for JS external defined in libraries.yml file, declaring library to be "external" does the job , like this

fluffines:
  js:
    'https://example.com/fluffines.min.js': { type: external }

Dug the sources a bit, and it looks like without the locale module, uris with stream wrappers are no problem.
But _locale_parse_js_file() coughs on any stream wrapper:

  // If there is still a protocol component in the path, reject that.
  if (strpos($filepath, ':')) {
    throw new Exception('Only local files should be passed to _locale_parse_js_file().');
  }

Rejecting all uris is clearly wrong and i wonder what to do. Beneath public://, there will be module://, and there are legitimate use cases for s3://, data:, phar:// and whatnot.

Given that, we may either cough on http(s), or remove the check altogether. As we only receive js files marked as 'file', we may do the latter. Otherwise, no harm in being conservative.

Patch flying in that coughs only on http(s) and includes the offending filename in the exception.

I was building a custom animations module (users with trusted permission levels can upload custom css/js on a custom entity type) and the patch from #19 was required in order to build dynamic libraries using those uploaded files. Once applied, everything started working as expected.

RTBC +1 from me.

Is it possible to add test coverage for this?

Adding some tests

Here is a test only patch (with some X files related strings) and an updated patch.

No interdiff, since that is literally the test :)

Huh, did not know about the void typehint in ::setup

Attached the same test only patch here so its easier to review without jumping back and forth.

Right, that was the test only patch

Thanks for the test, I'd suggest to add some more cases to explicitly test for "not allowed" protocols like http(s), ftp(s), and //.

IMO when public:// files are actually resided locally, then _locale_parse_js should try to properly resolve them. Then, the current (unpatched) exception handling also would make more sense to me.

Is #2385069: Locale JS alter breaks on remote JS files a duplicate of this?

The patch over there is using UrlHelper::isExternal - is that a better approach than using preg_match?

The issue over there has no tests, so if that one is indeed a duplicate, we should close it in favour of this one

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rerolled patch #15 to work with version 2.16.0

wrong issue.

#28 work for me, drupal 9.5.9 and asset injector 8.x-2.17.
THX

Hiding #39 as it veers off course. #28 is the patch to keep pushing along. And this is still NW for adding more test coverage for HTTP(s) urls in the test coverage.

#2385069: Locale JS alter breaks on remote JS files was closed a duplicate, moving over credit

I've created a MR based on patch #2735717-28: Stream wrapper reference in JS library causes error in _locale_parse_js_file()

needs more work to fix tests

Hello, I've updated the fork with latest code, updated the test adding annotation and fixing some phpcs and, since this issue is considered duplicated of #2385069: Locale JS alter breaks on remote JS files I am adding this piece of code !UrlHelper::isExternal($item['data']), for external js files to not be passed through the locale_js_translate function:

  #[Hook('js_alter')]
  public function jsAlter(&$javascript, AttachedAssetsInterface $assets, LanguageInterface $language): void {
    $files = [];
    foreach ($javascript as $item) {
      if (isset($item['type']) && $item['type'] == 'file' && !UrlHelper::isExternal($item['data'])) {

The only test failing seems not to be related with this issue, but i am not sure about it:

Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTest::testInlineBlocksRevisioning
TypeError: str_starts_with(): Argument #1 ($haystack) must be of type string, array given