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

Fix assets path issue for "compound-design-tokens" package #26636

Closed
wants to merge 1 commit into from
Closed

Fix assets path issue for "compound-design-tokens" package #26636

wants to merge 1 commit into from

Conversation

menturion
Copy link

@menturion menturion commented Nov 24, 2023

The fix, given here #26069, does not work if one clone the compound-design-tokens package separately and link it within the matrix-react-sdk, because the regex /@vector-im(?:\\|\/)compound-(.*?)(?:\\|\/)/ does not match the different paths that does not contain the string @vector-im, see below (Cloned & linked package):

Resource paths (on Windows) are e.g. ...

  • Not linked package -
    resourcePath: C:\src\matrix-react-sdk\node_modules\@vector-im\compound-design-tokens\icons\chat-solid.svg

  • Cloned & linked package -
    resourcePath: C:\src\compound-design-tokens\icons\chat-solid.svg

Problem:
The main problem is not whether absolute vs relative paths but that the compound-design-tokens package does not have a dist or res subfolder to serve the assets so that the existing regex /^.*[/\\](dist|res)[/\\]/ does not match.

Solution:
The solution is to add the missing root folder name for the assets to the above regex, with ...
/^.*[/\](dist|res|compound-design-tokens)[/\]/

I have tested this for both scenarios (linked/not linked) on Windows and it works with the regex statement:

const prefix = /^.*[/\\](dist|res|compound-design-tokens)[/\\]/;

The full part regarding compoundImportsPrefix is in this case obsolete and can be removed in the webpack.config.js.

Signed-off-by: menturion menturion@gmail.com


This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Fixed path issue for "compound-design-tokens" package independent of whether the package is cloned and linked within "matrix-react-sdk" or not.

Resource paths are e.g. ...

- Not linked package -
resourcePath: C:\src\matrix-react-sdk\node_modules\@vector-im\compound-design-tokens\icons\chat-solid.svg

- Cloned & linked package -
resourcePath: C:\src\compound-design-tokens\icons\chat-solid.svg

Problem: 
The "compound-design-tokens" package does not have a 'dist' or "res" subfolder for the assets so that the regex /^.*[/\\](dist|res)[/\\]/ does not match. 

Solution: 
Add the missing root folder name for the compound tokens assets to the regex in the "webpack.config.js" to match the paths, with resulting regex... 

/^.*[/\\](dist|res|compound-design-tokens)[/\\]/

Tested both scenarios (linked/not linked) on Windows.
@menturion menturion requested a review from a team as a code owner November 24, 2023 10:33
Copy link

Thanks for opening this pull request, unfortunately we do not accept contributions from the main branch of your fork, please re-open once you switch to an alternative branch for everyone's sanity. See https://github.com/matrix-org/matrix-js-sdk/blob/develop/CONTRIBUTING.md

@github-actions github-actions bot closed this Nov 24, 2023
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Nov 24, 2023
@menturion
Copy link
Author

Created a new PR #26646.
Sorry, my bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant