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.

Sign up for GitHub

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

Jump to bottom

Typescript : Property '#private' in type 'PHP' refers to a different member that cannot be accessed from within type 'PHP'. #1577

Closed
mho22 opened this issue Jul 7, 2024 · 3 comments · Fixed by #1585
Closed

Typescript : Property '#private' in type 'PHP' refers to a different member that cannot be accessed from within type 'PHP'. #1577

mho22 opened this issue Jul 7, 2024 · 3 comments · Fixed by #1585
Assignees
@adamziel
Labels
[Priority] High [Type] Bug An existing feature does not function as intended

Comments

@mho22
Copy link
Contributor

mho22 commented Jul 7, 2024 •
edited
Loading

With new behavior :

import { PHP } from '@php-wasm/universal';
import { loadNodeRuntime, useHostFilesystem } from '@php-wasm/node';


const php : PHP = new PHP( await loadNodeRuntime( '8.0' ) );

useHostFilesystem( php );

An error occurs with object php in useHostFilesystem :

Argument of type 'import("project/node_modules/@php-wasm/universal/lib/php").PHP' is not assignable to parameter of type 'PHP'.
  Property '#private' in type 'PHP' refers to a different member that cannot be accessed from within type 'PHP'.
 ts(2345)
const php: PHP

With new behavior :

import { PHP } from '@php-wasm/universal';
import { loadNodeRuntime, useHostFilesystem } from '@php-wasm/node';


const php : PHP = new PHP( await loadNodeRuntime( '8.0' ) );

useHostFilesystem( php );

php.mount( process.cwd(), createNodeFsMountHandler( process.cwd() ) );

An error occurs with method createNodeFsMountHandler() in php.mount() :

Argument of type 'import("/Users/mho/Work/Projects/Development/Web/Side/browser-php/node_modules/@php-wasm/node/index").MountHandler' is not assignable to parameter of type 'import("/Users/mho/Work/Projects/Development/Web/Side/browser-php/node_modules/@php-wasm/universal/lib/php").MountHandler'.
  Types of parameters 'php' and 'php' are incompatible.
    Type 'import("/Users/mho/Work/Projects/Development/Web/Side/browser-php/node_modules/@php-wasm/universal/lib/php").PHP' is not assignable to type 'PHP'.ts(2345)
(alias) createNodeFsMountHandler(localPath: string): MountHandler
import createNodeFsMountHandler
@adamziel adamziel mentioned this issue Jul 8, 2024
Open
adamziel added a commit that referenced this issue Jul 8, 2024
@adamziel
php-wasm/node: Ship as ESM and CJS
b240066 
The `@php-wasm/node` package used to be CommonJS only. This created
tricky dependency identity problems in ESM applications, e.g. in #1577
the main script imports the ESM version of `@php-wasm/universal`, but
`@php-wasm/node` imports the CJS version of it.

This PR ensures `@php-wasm/node` has an ESM version available.

This is first step towards solving #1583

Closes #1579
Closes #1577

 ## Testing instructions

Run `npm run build`, copy the built packages from `dist/packages/php-wasm` to
node_modules in another directory, create a test.js script with the
following contents:

```
import { PHP } from '@php-wasm/universal';
import { loadNodeRuntime, useHostFilesystem } from '@php-wasm/node';

const php = new PHP(await loadNodeRuntime('8.0'));
console.log("Hey");
console.log(await php.run({ code: 'echo "Hello, World!";' }));
```

Then create a package.json file with `{"type":"module"}` and confirm
that running test.js works. Then create test.cjs file as follows:

```js
const { PHP } = require('@php-wasm/universal');
const { loadNodeRuntime } = require('@php-wasm/node');

async function main() {
    const runtimeId = await loadNodeRuntime('8.0');
    console.log({runtimeId})
    const php = new PHP(runtimeId);

    console.log(php);
}
main();
```

And confirm this one works, too.
@adamziel adamziel mentioned this issue Jul 8, 2024
Merged
@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Priority] High labels Jul 8, 2024
@adamziel adamziel self-assigned this Jul 8, 2024
@adamziel
Copy link
Collaborator

adamziel commented Jul 9, 2024

This works as expected now in version 0.9.18
@mho22
Copy link
Contributor Author

mho22 commented Jul 9, 2024

@adamziel I am not sure if this is correct, but it seems this issue is still relevant.

I can find in both files :

export declare class PHP implements Disposable {
	#private;
	protected [__private__dont__use]: any;
	requestHandler?: PHPRequestHandler;
	...

I still get the error :

Argument of type 'import("/Users/mho/Work/Projects/Development/Web/Side/browser-php/node_modules/@php-wasm/universal/index").PHP' is not assignable to parameter of type 'PHP'.

Do you think this is related ?
@adamziel adamziel mentioned this issue Jul 9, 2024
Merged
@adamziel
Copy link
Collaborator

adamziel commented Jul 9, 2024

@mho22 aha, good spot! It's related to one of the PRs involved in fixing this issue – I just opened #1593 to correct that.
adamziel added a commit that referenced this issue Jul 16, 2024
@adamziel @brandonpayton
Build: Ship the default TypeScript .d.ts declaration files, not rollu…
56e4b0c 
…ps (#1593)

pipeline. This caused the published packages to ship not just their own
types but also all the types they import. This creates a lot of noise
and also type identity conflicts between packages. This PR removes most
of these rollup tasks, leaving only two:

* In the `@wp-playground/blueprints` package for Types->JSON schema
generation
* In the `@wp-playground/client` package to make it self-contained and
dependencyless.

Related to #1577

 ## Testing instructions

Run `npm run build` and confirm the built packages only export their own
types.

---------

Co-authored-by: Brandon Payton <brandon@happycode.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Priority] High [Type] Bug An existing feature does not function as intended
2 participants
@adamziel @mho22