Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ php-wasm ] : express dependency update #1882

Open
wants to merge 16 commits into
base: trunk
Choose a base branch
from

Conversation

mho22
Copy link
Contributor

@mho22 mho22 commented Oct 10, 2024

Motivation for the change, related issues

Related to issue #1855

Implementation details

Updated express from version 4.19.2 to 4.21.1

Testing Instructions (or ideally a Blueprint)

npm run test

Addition informations

Should I also clean some unused dependencies like deprecated @types/ajv and rename the pull request ? Like I suggested in the issue.

@mho22 mho22 requested a review from a team as a code owner October 10, 2024 12:59
@mho22
Copy link
Contributor Author

mho22 commented Oct 10, 2024

Alright, something is very bizarre here. As you can see every commits I made : something went wrong with the package @zkochan/js-yaml in package-lock.json :

npm error Invalid: lock file's @zkochan/[email protected] does not satisfy @zkochan/[email protected]
npm error Missing: @zkochan/[email protected] from lock file

no matter what I did, the @nx/eslint package kept the version 0.0.7 in its peerDependencies provoking a Failed Lint and typecheck check 6 times in a row.

I manually modified the version of @zkochan/js-yaml and the first CI check passed.

"node_modules/@nx/eslint": {
	"version": "19.8.4",
        ...
	"dependencies": {
		"@nx/devkit": "19.8.4",
		"@nx/js": "19.8.4",
		"@nx/linter": "19.8.4",
		"semver": "^7.5.3",
		"tslib": "^2.3.0",
		"typescript": "~5.4.2"
	},
	"peerDependencies": {
-		"@zkochan/js-yaml": "0.0.7",
+		"@zkochan/js-yaml": "0.0.6",
		"eslint": "^8.0.0 || ^9.0.0"
	},
	...

So, I think something is broken somewhere. I only updated express dependency from 4.19.2 to 4.21.1. Nothing related to @nx/eslint.

The Failing CI check failed on some line linked with Docusaurus :

[INFO] [en] Creating an optimized production build...
[ERROR] Unable to build website for locale en.
[ERROR] TypeError: TypeDoc__namespace.Application.bootstrapWithPlugins is not a function
at Object.generateJson (/home/runner/work/wordpress-playground/wordpress-playground/node_modules/docusaurus-plugin-typedoc-api/lib/plugin/data.js:73:52)

I don't think this should be merged into trunk yet. I don't know why I got that problem and how to correct it correctly. What do you guys think about this ?

@adamziel
Copy link
Collaborator

So weird! That package doesn't even seem to be a dependency of express. Perhaps upgrading @nx would help? Unfortunately, touching some of these packages causes a cascade of interconnected updates.

@mho22
Copy link
Contributor Author

mho22 commented Oct 18, 2024

@adamziel Ok, this was not easy at all, but ! I almost made it.

First, the steps :

I did what you said : Update @nx. And this is what happened :

Update NX > Update Typescript Eslint > Update Eslint > Update Vite > Update Vitest > Update SWC > Update Rollup

I had to modify some packages that were deprecated :

vite-plugin-api -> vite-plugin-api-routes
@vitest/coverage-c8 -> @vitest/coverage-v8
@nx/linter -> @nx/eslint
rollup-plugin-ts -> rollup-plugin-typescript

I needed to downgrade a package because of ESLint Flat config support only [ I think ] :

eslint-plugin-cypress -> 4.0.0 -> 3.6.0

And I made use of this PR to remove the unused package @types/ajv


I then had to update files :

  1. Modified every project.json file with @nx/eslint:lint instead of @nx/linter
  2. Modified every vite.config.js by removing deprecated cache.dir
  3. Removed deprecated tasksRunnerOptions value in nx.json
  4. Replaced vite-ts-config-paths with current plugin in playground/website/vite.config.ts and replaced viteTsConfigPaths with tsConfigPaths
  5. Modified packages/playground/website/package.json type : commonjs in type module
  6. Manipulated UINT8Array in tests to match with expected data. [ Probably 40 lines in tests needed to be updated ]

I got this message with Node 22, but none with Node 20 :

(node:86254) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

I still got errors from test on local but probably not related to the upgrade :

  • playground-php-cors-proxy -> phpunit: command not found
  • playground-blueprints -> Timeouts
  • php-wasm-node :
- php.spec.ts : Should work with long POST body : new URL('./test-data/long-post-body.txt', import.meta.url).pathname -> file not found
- php-crash.spec.ts :Does not leak memory when creating and destroying instances : "global.gc" does not exist even with --expose-gc

 
 

If this is working as expected, in the end, there will be not much more to do to have an "up-to-date" project dependency list 🤞. I think.

 
 

Edit: It looks like there are 66 lint errors, most of them in files I haven't modified. I'll need to check this out.

Suggestions :

  1. It seems vite-tsconfig-paths is working now. vite-extensions/vite-ts-config-paths.ts is maybe not needed anymore, should I remove this from every vite.config.ts files and vite-extensions directory ?

@mho22
Copy link
Contributor Author

mho22 commented Oct 21, 2024

@adamziel I corrected the lint errors and tried multiple times to make the tests succeed. But :

test-unit-asyncify :

❌ > nx run playground-blueprints:"test:vite"
❌ > nx run php-wasm-node:test

Probably for the same reasons I mentionned above [ timeouts and a file not found ]

test-e2e :

  Query API
    option `php`
      1) should load PHP 8.0 by default
      2) should load PHP 7.4 when requested
    option `wp`
      3) should load WordPress latest by default
      4) should load WordPress 6.3 when requested
    option `networking`
      5) should disable networking when requested
      6) should enable networking when requested
      7) should return true from wp_http_supports(array( "ssl" ))
    option `plugin`
      8) should install the specified plugin
    option `theme`
      9) should install the specified theme
    option `url`
      10) should load the specified URL
    option `mode`
      11) lack of mode=seamless should a WordPress in a simulated browser UI
      12) mode=seamless should load a fullscreen WordPress
    option `login`
      13) should log the user in as an admin
      14) should not log the user in as an admin when not requested
    option `multisite`
      15) should enable a multisite
    option `lazy`
      16) should defer loading the Playground assets until someone clicks on the "Run" button
    Patching Gutenberg editor frame
      17) should patch the editor frame in WordPress
      18) should patch the editor frame in Gutenberg when it is installed as a plugin
  0 passing (4s)
  18 failing

Cypress is furious. Nothing is found. A disaster. And I can't find why.

build :

nx run docs-site:build


[ERROR] Docusaurus server-side rendering could not render static page with path /blueprints/steps.
[ERROR] Docusaurus server-side rendering could not render static page with path /developers/apis/javascript-api/playground-api-client.

  
  TypeError: Cannot read properties of undefined (reading 'filter')
  Error: Invalid API member: @wp-playground/client.PlaygroundClient.run. Node 'PlaygroundClient' not found in node '@wp-playground/client'.}

I tried to update docusaurus but the errors remained. And it is really difficult to know why these errors occur.

the next errors are timeouts.

At this point, I don't know how to make these tests valid. Any insights ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Reviewed
Development

Successfully merging this pull request may close these issues.

2 participants