{"action":"created","issue":{"url":"https://api.github.com/repos/apache/cordova-lib/issues/602","repository_url":"https://api.github.com/repos/apache/cordova-lib","labels_url":"https://api.github.com/repos/apache/cordova-lib/issues/602/labels{/name}","comments_url":"https://api.github.com/repos/apache/cordova-lib/issues/602/comments","events_url":"https://api.github.com/repos/apache/cordova-lib/issues/602/events","html_url":"https://github.com/apache/cordova-lib/pull/602","id":267897815,"node_id":"MDExOlB1bGxSZXF1ZXN0MTQ4Mjk3NzY1","number":602,"title":"CB-12774 : Don't munge scoped plugin IDs anymore. Comment requested.","user":{"login":"akdor1154","id":6732831,"node_id":"MDQ6VXNlcjY3MzI4MzE=","avatar_url":"https://avatars0.githubusercontent.com/u/6732831?v=4","gravatar_id":"","url":"https://api.github.com/users/akdor1154","html_url":"https://github.com/akdor1154","followers_url":"https://api.github.com/users/akdor1154/followers","following_url":"https://api.github.com/users/akdor1154/following{/other_user}","gists_url":"https://api.github.com/users/akdor1154/gists{/gist_id}","starred_url":"https://api.github.com/users/akdor1154/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akdor1154/subscriptions","organizations_url":"https://api.github.com/users/akdor1154/orgs","repos_url":"https://api.github.com/users/akdor1154/repos","events_url":"https://api.github.com/users/akdor1154/events{/privacy}","received_events_url":"https://api.github.com/users/akdor1154/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2017-10-24T04:58:32Z","updated_at":"2018-09-05T10:20:29Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/apache/cordova-lib/pulls/602","html_url":"https://github.com/apache/cordova-lib/pull/602","diff_url":"https://github.com/apache/cordova-lib/pull/602.diff","patch_url":"https://github.com/apache/cordova-lib/pull/602.patch"},"body":"In order to get plugins that are under an npm scope (e.g. `@akdor1154/some-plugin`) to install, a previous change from took the approach to consider such a plugin to be called (have the id) `some-plugin` instead of `@akdor1154/some-plugin`. This allowed scoped plugins to install and preserved the assumption that plugins will always be installed in `plugins_dir/[plugin_id]`.\r\n\r\nHowever, it required special parsing logic around `npm` package IDs, and it broke the assumption that the `name` in an npm plugin's `package.json` would correspond to Cordova's idea of a plugin ID. IMO this approach is the source of further complexity which is not required, and is leading to weird bugs and special cases with scoped plugins. (see the linked issue, but there is stuff as basic as \"`npm install` no longer works after installing a scoped plugin\")\r\n\r\nThis PR changes approach - such plugins are now considered to have the id `@akdor1154/some-plugin`, in agreement with how npm treats such packages. This allows almost all special cases for scoped packages to be removed (yay). The key difference in behaviour as a result of this, though, is that while plugins are still installed in `plugins_dir/[plugin_id]`, `[plugin_id]` may no longer be a single directory, leading to plugin directories that look like\r\n```\r\nplugins\r\n|- @akdor1154\r\n|   |- some-plugin\r\n|- cordova-some-other-plugin\r\n```\r\n\r\nMost of the logic changes in this PR are based around making this change work.\r\n\r\nIt's largely done how I want it, but I guess maintainers probably have strong opinions over whether this is the right way to go or not. Because of this I've left some commits in marked as TEMP that are for my own workflow (i.e. I am expecting to have to make further changes). Please keep in mind I'll remove these. The only one you should be mindful of is the monkey patch to `cordova-common`; this would need to be raised in a separate PR to cordova-common if this change was approved in principle.\r\n\r\nThe work was done on a Mac but I'll check it properly on my windows laptop when I get home (path sep issues are expected).\r\n\r\n\r\n### Platforms affected\r\nAll\r\n\r\n### What testing has been done on this change?\r\nNew end-to-end integration tests including a proper scoped plugin fixture\r\nUnit tests on scoped plugins\r\nUnit tests for plugman metadata\r\n\r\n### Checklist\r\n- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database\r\n- [x] Commit message follows the format: \"CB-3232: (android) Fix bug with resolving file paths\", where CB-xxxx is the JIRA ID & \"android\" is the platform affected.\r\n- [x] Added automated test coverage as appropriate for this change.\r\n\r\ncc: @riknoll "},"comment":{"url":"https://api.github.com/repos/apache/cordova-lib/issues/comments/418679124","html_url":"https://github.com/apache/cordova-lib/pull/602#issuecomment-418679124","issue_url":"https://api.github.com/repos/apache/cordova-lib/issues/602","id":418679124,"node_id":"MDEyOklzc3VlQ29tbWVudDQxODY3OTEyNA==","user":{"login":"janpio","id":183673,"node_id":"MDQ6VXNlcjE4MzY3Mw==","avatar_url":"https://avatars0.githubusercontent.com/u/183673?v=4","gravatar_id":"","url":"https://api.github.com/users/janpio","html_url":"https://github.com/janpio","followers_url":"https://api.github.com/users/janpio/followers","following_url":"https://api.github.com/users/janpio/following{/other_user}","gists_url":"https://api.github.com/users/janpio/gists{/gist_id}","starred_url":"https://api.github.com/users/janpio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/janpio/subscriptions","organizations_url":"https://api.github.com/users/janpio/orgs","repos_url":"https://api.github.com/users/janpio/repos","events_url":"https://api.github.com/users/janpio/events{/privacy}","received_events_url":"https://api.github.com/users/janpio/received_events","type":"User","site_admin":false},"created_at":"2018-09-05T10:20:29Z","updated_at":"2018-09-05T10:20:29Z","author_association":"CONTRIBUTOR","body":"@tobilarscheid This PR has a failing test on Windows:\r\n\r\n```\r\nFailures:\r\n1) plugin end-to-end Test 013 : should be able to add and remove scoped npm packages without screwing up everything\r\n  Message:\r\n    Expected Plugin \"@testscope/cordova-lib-test-plugin-scoped\" is not present in the project. See `cordova plugin list`. to be undefined.\r\n  Stack:\r\n    Error: Expected Plugin \"@testscope/cordova-lib-test-plugin-scoped\" is not present in the project. See `cordova plugin list`. to be undefined.\r\n        at C:\\projects\\cordova-lib\\integration-tests\\plugin.spec.js:329:29\r\n        at _rejected (C:\\projects\\cordova-lib\\node_modules\\q\\q.js:797:24)\r\n        at C:\\projects\\cordova-lib\\node_modules\\q\\q.js:823:30\r\n        at Promise.when (C:\\projects\\cordova-lib\\node_modules\\q\\q.js:1035:31)\r\n```\r\n\r\nAlso unfortunately the branch is now also conflicted and has to be rebased. Could you take care of that @akdor1154 please? Thanks."}}
{"actor":{"display_login":"janpio"}}