Skip to content

js split - #224

Merged
justvanrossum merged 5 commits into
mainfrom
js-split
Mar 3, 2025
Merged

js split#224
justvanrossum merged 5 commits into
mainfrom
js-split

Conversation

@simoncozens

Copy link
Copy Markdown
Contributor
  • Correct paths
  • Add missing build hook
  • Oops, missing files

@justvanrossum

Copy link
Copy Markdown
Member

I get the same install failure as the CI when trying to pip install this locally.

@simoncozens

Copy link
Copy Markdown
Contributor Author

Yep, I need to update the URLs.

@justvanrossum

Copy link
Copy Markdown
Member

Any chance you can get this to work today? I'd really like to merge both this and the main PR.

@simoncozens

Copy link
Copy Markdown
Contributor Author

I will get this working for test purposes, but the JS dependencies will need to be adjusted once again when the Fontra js-split PR is merged.

@justvanrossum

Copy link
Copy Markdown
Member

This works, thanks!

So, after the main js-split merge, we need to remove the js-split-2 branch from the gitpkg URLs, right?

I see "integrity" hashes in package-lock.json. Do these get invalidated after each push to the associated repo? Or how does this affect the management of the projects? Does dependabot pick these up?

@justvanrossum

Copy link
Copy Markdown
Member

I'm locally trying to edit the gitpkg URLs in package.json, but when I then run npm install and npm run bundle, the latter fails:

> bundle
> webpack --config webpack.config.cjs --mode production

assets by status 7.42 KiB [cached] 3 assets
Entrypoint __bundler-plugin-entry__landing = landing.html landing.css
Entrypoint landing = landing.b209169f8a8943e44146.js
runtime modules 670 bytes 3 modules
orphan modules 821 bytes [orphan] 1 module
cacheable modules 8.46 KiB
  ./src-js/projectmanager-rcjk/landing.html 2.09 KiB [built] [code generated]
  ./src-js/projectmanager-rcjk/src/landing.js + 1 modules 2.17 KiB [built] [code generated]
  ./node_modules/@fontra/projectmanager-filesystem/assets/landing.css 4.2 KiB [built] [code generated]

ERROR in ./node_modules/@fontra/projectmanager-filesystem/src/landing.js 1:0-54
Module not found: Error: Can't resolve '@fontra/core/backend-api.js' in '/Users/just/code/git/BlackFoundry/fontra-rcjk/node_modules/@fontra/projectmanager-filesystem/src'
 @ ./src-js/projectmanager-rcjk/src/landing.js 2:0-82 5:2-20

ERROR in ./node_modules/@fontra/projectmanager-filesystem/src/landing.js 2:0-63
Module not found: Error: Can't resolve '@fontra/core/loader-spinner.js' in '/Users/just/code/git/BlackFoundry/fontra-rcjk/node_modules/@fontra/projectmanager-filesystem/src'
 @ ./src-js/projectmanager-rcjk/src/landing.js 2:0-82 5:2-20

ERROR in ./node_modules/@fontra/projectmanager-filesystem/src/landing.js 3:0-40
Module not found: Error: Can't resolve '@fontra/core/theme-settings.js' in '/Users/just/code/git/BlackFoundry/fontra-rcjk/node_modules/@fontra/projectmanager-filesystem/src'
 @ ./src-js/projectmanager-rcjk/src/landing.js 2:0-82 5:2-20

ERROR in ./src-js/projectmanager-rcjk/src/landing.js 1:0-53
Module not found: Error: Can't resolve '@fontra/core/utils.js' in '/Users/just/code/git/BlackFoundry/fontra-rcjk/src-js/projectmanager-rcjk/src'

4 errors have detailed information that is not shown.
Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.

 HTML Bundler Plugin   ▶▶▶ (webpack 5.97.1) compiled with 4 errors in 161 ms

As usual, I may be doing something wrong or misunderstand something.

@simoncozens

Copy link
Copy Markdown
Contributor Author

So, after the main js-split merge, we need to remove the js-split-2 branch from the gitpkg URLs, right?

Right, yes.

I'm locally trying to edit the gitpkg URLs in package.json, but when I then run npm install and npm run bundle, the latter fails.

This happened for me; I trashed the package-lock.json and tried again, and it worked.

@justvanrossum

Copy link
Copy Markdown
Member

Hm, I did:

diff --git a/src-js/projectmanager-rcjk/package.json b/src-js/projectmanager-rcjk/package.json
index b579a75..7e533cd 100644
--- a/src-js/projectmanager-rcjk/package.json
+++ b/src-js/projectmanager-rcjk/package.json
@@ -10,7 +10,7 @@
     "projectmanager": "rcjk"
   },
   "dependencies": {
-    "@fontra/core": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?js-split-2",
-    "@fontra/projectmanager-filesystem": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?js-split-2"
+    "@fontra/core": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?main",
+    "@fontra/projectmanager-filesystem": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?main"
   }
 }

Then

  • rm package-lock.json
  • npm install
  • npm run bundle

But keep getting the same error.

(Does gitpkg.vercel.app update instanteneous, or could there be a delay, and it may still see the pre-split version?)

@justvanrossum

Copy link
Copy Markdown
Member

Ah, it does work if I replace the branch with a specific commit.

@justvanrossum

Copy link
Copy Markdown
Member

This would be the total diff after npm install:

diff --git a/package-lock.json b/package-lock.json
index 731e711..ef363bb 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -33,8 +33,8 @@
       }
     },
     "node_modules/@fontra/core": {
-      "resolved": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?js-split-2",
-      "integrity": "sha512-ple17lpINSpmaQoMg5/MsbhknJJYVXp1Num+gNIqwUO7nwLs6naqUEEGY2RKgxkmekdPiG9wwPgXAbmLCj5FMw==",
+      "resolved": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?c604e72ad7c7b05b24848c50d253016f6e091c6a",
+      "integrity": "sha512-DOucz16A+mQIWS268snlqtwkkC1cxRjEntdssRacSd8ovozp2bbOqyH2VCD9Bl56FmRI3egLGthMzR9ZH3ikiw==",
       "dependencies": {
         "@fontra/core": "file:",
         "bezier-js": "^6.1.4",
@@ -45,8 +45,8 @@
     },
     "node_modules/@fontra/projectmanager-filesystem": {
       "version": "0.0.1",
-      "resolved": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?js-split-2",
-      "integrity": "sha512-kAjovyBM980oeAovQVHmjq6Nh1R2jklsql6ukRn+KS9tzXb/TMDvULHmMK6tjxNu0rMbo+hy4Q9R+qOrvPsBsg=="
+      "resolved": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?c604e72ad7c7b05b24848c50d253016f6e091c6a",
+      "integrity": "sha512-r3DI0zSI1WwDiyqE42+gBDuPtrMIeybZ8UgIKAWJs8LQPW4R4rDn238vJ5tZevNcRHufYG2aP5UGENblJ4iLRg=="
     },
     "node_modules/@fontra/projectmanager-rcjk": {
       "resolved": "src-js/projectmanager-rcjk",
@@ -5189,8 +5189,8 @@
       "name": "@fontra/projectmanager-rcjk",
       "version": "0.0.1",
       "dependencies": {
-        "@fontra/core": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?js-split-2",
-        "@fontra/projectmanager-filesystem": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?js-split-2"
+        "@fontra/core": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?c604e72ad7c7b05b24848c50d253016f6e091c6a",
+        "@fontra/projectmanager-filesystem": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?c604e72ad7c7b05b24848c50d253016f6e091c6a"
       }
     }
   }
diff --git a/src-js/projectmanager-rcjk/package.json b/src-js/projectmanager-rcjk/package.json
index b579a75..3fdbe54 100644
--- a/src-js/projectmanager-rcjk/package.json
+++ b/src-js/projectmanager-rcjk/package.json
@@ -10,7 +10,7 @@
     "projectmanager": "rcjk"
   },
   "dependencies": {
-    "@fontra/core": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?js-split-2",
-    "@fontra/projectmanager-filesystem": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?js-split-2"
+    "@fontra/core": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?c604e72ad7c7b05b24848c50d253016f6e091c6a",
+    "@fontra/projectmanager-filesystem": "https://gitpkg.vercel.app/googlefonts/fontra/src-js/projectmanager-filesystem?c604e72ad7c7b05b24848c50d253016f6e091c6a"
   }
 }

@justvanrossum

Copy link
Copy Markdown
Member

The question remains on how do we best maintain this.

@justvanrossum

Copy link
Copy Markdown
Member

(Let me know if you want me to commit it in this state.)

@simoncozens

Copy link
Copy Markdown
Contributor Author

I think https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?main would work.

@justvanrossum

Copy link
Copy Markdown
Member

I think https://gitpkg.vercel.app/googlefonts/fontra/src-js/fontra-core?main would work.

Except it didn't, and doesn't :( Hence my question about possible delay in gitpkg updating.

@justvanrossum

Copy link
Copy Markdown
Member

So what do you recommend we'd do? I want to merge this PR today, too.

@justvanrossum

Copy link
Copy Markdown
Member

It seems that the redirect to login (if not logged in) no longer works. Not sure if that is here or in the main fontra repo.

@simoncozens

Copy link
Copy Markdown
Contributor Author

I think I have a fix, just testing it.

@simoncozens

Copy link
Copy Markdown
Contributor Author

OK, there we go, npm was going mad about these packages not having proper versions, and npm install --prefer-dedupe (for some reason) fixes it.

@justvanrossum

Copy link
Copy Markdown
Member

Works for me. However, if I remove the --prefer-dedupe option, it still works. So I wonder again if we were fooled by a delayed update by gitpkg.vercel?

@justvanrossum

Copy link
Copy Markdown
Member

Other than that, is this good to go for you?

@justvanrossum

justvanrossum commented Mar 3, 2025

Copy link
Copy Markdown
Member

It seems that the redirect to login (if not logged in) no longer works. Not sure if that is here or in the main fontra repo.

Not here, but in the main repo: https://github.com/googlefonts/fontra/issues/2074

@simoncozens

Copy link
Copy Markdown
Contributor Author

Works for me. However, if I remove the --prefer-dedupe option, it still works. So I wonder again if we were fooled by a delayed update by gitpkg.vercel?

No, we resolved the dependency problem and put the resolution into package.json so it doesn't need to think about it any more.

@simoncozens

Copy link
Copy Markdown
Contributor Author

Other than that, is this good to go for you?

Fine for me, I'll fix the redirect problem separately.

@justvanrossum

Copy link
Copy Markdown
Member

No, we resolved the dependency problem and put the resolution into package.json so it doesn't need to think about it any more.

The fact remains that if I remove the --prefer-dedupe option, and delete the package-lock.json file, pip install -e . just works, and I see no diff in package-lock.json.

@justvanrossum
justvanrossum marked this pull request as ready for review March 3, 2025 20:33
@justvanrossum
justvanrossum merged commit 5eb4c3a into main Mar 3, 2025
@justvanrossum
justvanrossum deleted the js-split branch March 3, 2025 20:33
@justvanrossum

Copy link
Copy Markdown
Member

Urgh, now the main branch fails, despite the PR was all green.

@simoncozens

Copy link
Copy Markdown
Contributor Author

Will fix in the morning.

@justvanrossum

Copy link
Copy Markdown
Member

I'm pretty sure this is what's happening:

I see "integrity" hashes in package-lock.json. Do these get invalidated after each push to the associated repo?

"Yes".

So, how can we organize this better?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants