-
-
Notifications
You must be signed in to change notification settings - Fork 296
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: enable appium-inspector as an appium plugin #1878
Conversation
* make inspector as appium plugin * added review comments
* apply format and lint * update lock
@@ -34,16 +34,19 @@ | |||
"build": "npm run build:browser && npm run build:electron", | |||
"build:browser": "vite build", | |||
"build:browser:url": "vite build --base $PUBLIC_URL", | |||
"build:plugin": "vite build --base /inspector --outDir ../../plugins/dist-browser", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be a part of the common build
script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to include this in build
after completing npm publish work as well, so yes but not yet in this PR.
"build:electron": "electron-vite build", | ||
"preview:browser": "npm run build:browser && vite preview", | ||
"preview:electron": "electron-vite preview", | ||
"pack:electron": "electron-builder build --publish never", | ||
"clean": "npm run clean:electron && npm run clean:browser && npm run clean:npm", | ||
"clean:electron": "rimraf dist/ && rimraf node_modules/.vite/ && rimraf node_modules/.vite-electron-renderer/", | ||
"clean:browser": "rimraf dist-browser/ && rimraf node_modules/.vite/", | ||
"clean:plugin": "rimraf plugins/dist-browser", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
"clean:npm": "rimraf package-lock.json && rimraf node_modules && npm install", | ||
"build:docs": "appium-docs build", | ||
"dev:docs": "appium-docs build --serve", | ||
"plugin:sync:version": "node ./scripts/sync-plugin.mjs", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
|
||
(TODO: add this release steps in .github/workflows/package.yml later as another PR) | ||
|
||
1. Run `npm run plugin:sync:version` to sync the version with the root project.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say this script should be executed automatically as soon as we bump the version of the main project via npm version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It will be a follow-up PR to treat npm publish stuff in my current plan.
When I activate the plugin and open the inspector page, Appium logs all the HTTP requests, which is where I am observing the 404 response:
|
I see. Then, maybe somewhere may have hard code to appium-inspector/app/web/polyfills.js Lines 5 to 8 in d4c6099
|
It is confusing that the locales load fine for you, though 🤔 maybe the translations are cached? If I open a new private window/incognito, the cache is cleared and I get the 404 again. |
This seems to do the trick and works correctly in both web and plugin builds: // Adjust locales path depending on Vite base (web vs plugin)
const viteBase = import.meta.env.BASE_URL;
const vitePath = viteBase.endsWith('/') ? viteBase : `${viteBase}/`;
const localesPath =
process.env.NODE_ENV === 'development'
? '/locales' // 'public' folder contents are served at '/'
: `..${vitePath}locales`; // from 'dist-browser/assets/' |
Yea, my env could be the browser cache reason. |
Complete partially in appium/appium#20834
Final PR instead of #1868, which was ordinally just for a prototype.
After this PR merge, we should run
npm publish
as README'srelease
section in this PR if needed for now. I'll also create a follow-up PR to make it automated as part of the existing workflow https://github.com/appium/appium-inspector/blob/main/.github/workflows/package.yml to push the npm package as well./inspector
endpointnpm
@appium/inspector-plugin
orappium-inspector-plugin
, which is better naming...? <- this will be in the follow-up PR