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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(patchers): fix patching modules resolving by require.resolve #240

Merged
merged 1 commit into from Jun 22, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jun 22, 2023

An alternative solution to fix problems mentioned in #239

Previously for patched modules in patching it uses absolute paths to the original modules like

import axios from '../../node_modules/@nextcloud/axios/dist/index.esm.js'

export { getRequestToken } from '@talk/node_modules/@nextcloud/auth/dist/index.esm.js'

It breaks if the absolute path is changed inside the module. Mentiond PR fixes the problem for spreed@stable27+ but breaks for spreed@stable26. Because of the absolute path, it may happen with any module at any time.


New solution resolves a real path by require.resolve and provides it by new aliases, for example

import axios from '@desktop-modules--@nextcloud/axios'

export { getRequestToken } from '@talk-modules--@nextcloud/auth'

Also, moved the patchers so their path looks like a path to the corresponding npm module.


Thanks @Antreesy for finding and explaining the problem 馃挋

@ShGKme ShGKme added bug Something isn't working 4. to release labels Jun 22, 2023
@ShGKme ShGKme self-assigned this Jun 22, 2023
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/noid/improve-patching-with-resolving branch from 71df855 to 2cbcc42 Compare June 22, 2023 16:34
@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 22, 2023

Rebased onto main

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Smoke tested on stable26, stable27, and master
No errors observed 馃Ν

@ShGKme ShGKme merged commit 2ed9834 into main Jun 22, 2023
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the fix/noid/improve-patching-with-resolving branch June 22, 2023 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants