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

feat: add beta for testing and npm available #1883

Merged
merged 8 commits into from
Jan 10, 2025
Merged

feat: add beta for testing and npm available #1883

merged 8 commits into from
Jan 10, 2025

Conversation

KazuCocoa
Copy link
Member

@KazuCocoa KazuCocoa commented Jan 6, 2025

This PR does:

Fixed

$ appium plugin install --source=npm [email protected]
✔ Checking if 'appium-inspector-plugin' is compatible
✖ Installing '[email protected]'
Error: ✖ Encountered an error when installing package: Package subpath './package.json' is not defined by "exports" in /Users/kazu/.appium/node_modules/appium-inspector-plugin/package.json

Fixed below as well, but I needed to add dependencies...

[Appium] Could not load plugin 'inspector', so it will not be available. Error in loading the plugin was: Cannot find package 'appium' imported from /Users/kazu/.appium/node_modules/appium-inspector-plugin/index.mjs
[Appium] Error: Cannot find package 'appium' imported from /Users/kazu/.appium/node_modules/appium-inspector-plugin/index.mjs
    at packageResolve (node:internal/modules/esm/resolve:854:9)
    at moduleResolve (node:internal/modules/esm/resolve:927:18)
    at defaultResolve (node:internal/modules/esm/resolve:1169:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:540:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:509:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:239:38)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:96:40)
    at link (node:internal/modules/esm/module_job:95:36)
[Appium] Welcome to Appium v2.14.1 (REV bab86d5de571015b63fd8fc30b47bbe072a1290e)
[Appium] Non-default server args:

I'll do below as another PR to merge this idea first:

@github-actions github-actions bot added the enhancement New feature or request label Jan 6, 2025
@KazuCocoa
Copy link
Member Author

@saikrishna321 Could you add https://www.npmjs.com/~kazucocoa to https://www.npmjs.com/package/appium-inspector-plugin project to push appium-inspector-plugin name space? Then, I'll add the package under https://www.npmjs.com/settings/appium/packages

If it would be nice to use a different name space as Appium, I'll rename this appium-inspector-plugin to another one to maintain it as Appium's npm channel. e.g. @appium/inspector-plugin as a 2nd candidate

@saikrishna321
Copy link
Member

@KazuCocoa Added you!

@KazuCocoa
Copy link
Member Author

Thank you!

@saikrishna321
Copy link
Member

@KazuCocoa Tested latest versions and works fine as plugin

@@ -1,7 +1,7 @@
import path from 'node:path';
import {fileURLToPath} from 'node:url';

import {BasePlugin} from 'appium/plugin.js';
import {BasePlugin} from '@appium/base-plugin';
Copy link
Member Author

@KazuCocoa KazuCocoa Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this way fixed:

[Appium] Could not load plugin 'inspector', so it will not be available. Error in loading the plugin was: Cannot find package 'appium' imported from /Users/kazu/.appium/node_modules/appium-inspector-plugin/index.mjs
[Appium] Error: Cannot find package 'appium' imported from /Users/kazu/.appium/node_modules/appium-inspector-plugin/index.mjs
    at packageResolve (node:internal/modules/esm/resolve:854:9)
    at moduleResolve (node:internal/modules/esm/resolve:927:18)
    at defaultResolve (node:internal/modules/esm/resolve:1169:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:540:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:509:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:239:38)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:96:40)
    at link (node:internal/modules/esm/module_job:95:36)
[Appium] Welcome to Appium v2.14.1 (REV bab86d5de571015b63fd8fc30b47bbe072a1290e)
[Appium] Non-default server args:

error...

appium plugin install --source=npm [email protected]

is only import {BasePlugin} from 'appium/plugin.js'; and peerDependencies. No "@appium/base-plugin": "^2.3.0" but appium --use-plugins=inspector--allow-cors cased error the above.

Latest appium plugin install --source=npm [email protected] is this current PR's latest.

(I know ideally we'd like to use import {BasePlugin} from 'appium/plugin.js'; only with peerDependencies)

@KazuCocoa KazuCocoa changed the title feat: add beta for testing feat: add beta for testing and npm available Jan 9, 2025
@KazuCocoa KazuCocoa marked this pull request as ready for review January 9, 2025 09:15
@eglitise
Copy link
Collaborator

eglitise commented Jan 10, 2025

Can the installation steps in the plugin's readme be updated?
I will update the full documentation in a separate PR.

@saikrishna321
Copy link
Member

@eglitise
Copy link
Collaborator

I'm aware; I was referring to the fact that it currently only lists the local installation steps. Since this PR adds the npm installation approach, it should also be added to the readme (and IMO the local installation steps can be removed, since they are objectively more complex for the end user, and belong in the Inspector documentation's Development docs)

@eglitise
Copy link
Collaborator

Installed beta.10 locally - works as expected 👍

Copy link
Collaborator

@eglitise eglitise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed in the description that the README update will come in another PR - all good then.

@KazuCocoa
Copy link
Member Author

Yea, initially, I thought I could put all the planned work in the description in one PR but I'd like to get review and merge only this PR since this was needed to fix Error in loading the plugin was: Cannot find package 'appium' imported from /Users/kazu/.appium/node_modules/appium-inspector-plugin/index.mjs error only via npm installation.
I'll do the rest of README work in upcoming another PR with GHA actions setup

@KazuCocoa KazuCocoa merged commit d11acc9 into main Jan 10, 2025
6 checks passed
@KazuCocoa KazuCocoa deleted the plugin-publish branch January 10, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants