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

Docs: Add npm dependency conflict in troubleshooting section #200

Merged
merged 6 commits into from
Jun 4, 2024

Conversation

hinakhadim
Copy link
Contributor

@hinakhadim hinakhadim commented Apr 2, 2024

As community members face npm dependency conflict issue when overriding header and footer, here I list down the points on how to resolve it. Then, include the link of this section in tutor core

README.rst Outdated
When there is a need to customize the ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer`` component, two things we have to care of to avoid dependency conflict error during ``npm`` installation.

1. Identify your openedx version, for example ``quince``.
2. Navigate to `learning <https://github.com/openedx/frontend-app-learning>`_ and `learner-dashboard <https://github.com/openedx/frontend-app-learner-dashboard>`_ MFEs repositories and checkout to branch ``open-release/quince.master``. Inspect which header and footer versions are installed from ``package.json``. You can explore the header and footer versions for additional MFEs to ensure that all MFEs support the same major versions of header and footer.
Copy link
Contributor

Choose a reason for hiding this comment

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

point 2 makes it seem like we are only focusing on learning and learner-dashboard MFE. The additional MFEs part does not stick. Maybe we can re-word this to focus that this applies to all MFEs but learning MFEs carry the most weight?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

README.rst Outdated
Comment on lines 511 to 564
7. Repeat the same process for customizing the footer component if necessary.


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can you add a stripped down version of example table you had on Google sheets? Visualizations help in conveying the message quickly.

README.rst Outdated
NPM Dependency Conflict When overriding ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer``
----------------------------------------------------------------------------------------------------------------

When there is a need to customize the ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer`` component, two things we have to care of to avoid dependency conflict error during ``npm`` installation.
Copy link
Contributor

Choose a reason for hiding this comment

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

not necessarily two things here.

@DawoudSheraz DawoudSheraz self-requested a review April 16, 2024 09:07
README.rst Outdated
NPM Dependency Conflict When overriding ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer``
----------------------------------------------------------------------------------------------------------------

When there is a need to customize the ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer`` component, it is necessary to address a crucial concerns to prevent dependency conflict errors during ``npm`` installation..
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a crucial concern

README.rst Outdated

When there is a need to customize the ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer`` component, it is necessary to address a crucial concerns to prevent dependency conflict errors during ``npm`` installation..

1. Identify your openedx version, for example ``quince``.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: for example,

2. Navigate to `learning <https://github.com/openedx/frontend-app-learning>`_ and `learner-dashboard <https://github.com/openedx/frontend-app-learner-dashboard>`_ MFEs repositories and checkout to branch ``open-release/quince.master``. Inspect which header and footer versions are installed from ``package.json``. This can also be applied to all MFEs to ensure consistency of versions but Learning and Learner Dashboard MFE carry the most weight.
3. Determine which versions of ``@edx/frontend-platform`` MFEs are utilizing and the header you plan to customize is compatible with the same version of ``@edx/frontend-platform`` specified in ``package.json`` file (peer-dependencies).
4. Ensure consistency between the versions. For example, If MFE has ``@edx/frontend-platform: 7.0.1``, then customize the header component which has ``@edx/frontend-platform: ^7.0.0`` in ``package.json`` under peer-dependencies
5. Checkout to that specific tag (e.g: ``v7.0.0``) of header component and customize it
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: header/footer component

README.rst Outdated
4. Ensure consistency between the versions. For example, If MFE has ``@edx/frontend-platform: 7.0.1``, then customize the header component which has ``@edx/frontend-platform: ^7.0.0`` in ``package.json`` under peer-dependencies
5. Checkout to that specific tag (e.g: ``v7.0.0``) of header component and customize it
6. Install the customized header/footer components into your MFEs. This will resolve any npm dependency conflict issues.
7. Repeat the same process for customizing the footer component if necessary.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: footer/header (considering in steps 1-6, we are using header/footer on various places, it is good to have the reverse in here).

README.rst Outdated
.. image:: https://raw.githubusercontent.com/overhangio/tutor-mfe/master/screenshots/npm-conflict-deps.png
:alt: Observation of MFE header and footer versions

From the above image, it can be observed that ``master`` branch of Learning MFE uses ``@edx/[email protected]`` and Discussoins MFE uses ``@edx/[email protected]``. If customized header is created from ``master`` branch, it ensures compatibility with the Discussions MFE as header module supports ``@edx/platform@^7.0.0``. However, The customized header triggers npm dependencies conflit error for learning MFE.
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussoins --> Discussions

README.rst Outdated
.. image:: https://raw.githubusercontent.com/overhangio/tutor-mfe/master/screenshots/npm-conflict-deps.png
:alt: Observation of MFE header and footer versions

From the above image, it can be observed that ``master`` branch of Learning MFE uses ``@edx/[email protected]`` and Discussoins MFE uses ``@edx/[email protected]``. If customized header is created from ``master`` branch, it ensures compatibility with the Discussions MFE as header module supports ``@edx/platform@^7.0.0``. However, The customized header triggers npm dependencies conflit error for learning MFE.
Copy link
Contributor

Choose a reason for hiding this comment

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

However, The customized --> However, the customized

README.rst Outdated
Comment on lines 517 to 564
In this case, checkout custom branch from ``v4.11.1`` of header for Learning MFE and ``v5.0.0`` for Discussions MFE.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can add more context to explain how these versions will work in build.

README.rst Outdated
4. Ensure consistency between the versions. For example, If MFE has ``@edx/frontend-platform: 7.0.1``, then customize the header/footer component which has ``@edx/frontend-platform: ^7.0.0`` in ``package.json`` under peer-dependencies
5. Checkout to that specific tag (e.g: ``v7.0.0``) of header component and customize it
6. Install the customized header/footer components into your MFEs. This will resolve any npm dependency conflict issues.
7. All the steps outlined above need to be followed for the footer as well, if you have followed them for the header
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add or vice versa at the end.

README.rst Outdated
NPM Dependency Conflict When overriding ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer``
----------------------------------------------------------------------------------------------------------------

When there is a need to customize the ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer`` component, it is necessary to address crucial concerns to prevent dependency conflict errors during ``npm`` installation.
Copy link
Contributor

Choose a reason for hiding this comment

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

it is necessary to address crucial concerns to prevent dependency conflict errors during ``npm`` installation.
☝🏽 this sentence can be removed and instead, something along the lines of there is a chance that npm dependency conflicts can occur. In the case of such a conflict, perform the following to resolve the conflicts while keeping the customizations in place:

README.rst Outdated
When there is a need to customize the ``@edx/frontend-component-header`` or ``@edx/frontend-component-footer`` component, it is necessary to address crucial concerns to prevent dependency conflict errors during ``npm`` installation.

1. Identify your openedx version, for example, ``quince``.
2. Navigate to `learning <https://github.com/openedx/frontend-app-learning>`_ and `learner-dashboard <https://github.com/openedx/frontend-app-learner-dashboard>`_ MFEs repositories and checkout to branch ``open-release/quince.master``. Inspect which header and footer versions are installed from ``package.json``. This can also be applied to all MFEs to ensure consistency of versions but Learning and Learner Dashboard MFE carry the most weight.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • nit: Learning and Learner Dashboard MFEs
  • While I remember I suggested mentioning Learner and learning MFEs carry the most weight (being learner-facing), it looks out of place context-wise. Maybe this can be re-written to point out that learner MFEs are mentioned only as an example. The step can be applied to all MFEs.

README.rst Outdated

1. Identify your openedx version, for example, ``quince``.
2. Navigate to `learning <https://github.com/openedx/frontend-app-learning>`_ and `learner-dashboard <https://github.com/openedx/frontend-app-learner-dashboard>`_ MFEs repositories and checkout to branch ``open-release/quince.master``. Inspect which header and footer versions are installed from ``package.json``. This can also be applied to all MFEs to ensure consistency of versions but Learning and Learner Dashboard MFE carry the most weight.
3. Determine which versions of ``@edx/frontend-platform`` MFEs are utilizing and the header/footer you plan to customize is compatible with the same version of ``@edx/frontend-platform`` specified in ``package.json`` file (peer-dependencies).
Copy link
Contributor

Choose a reason for hiding this comment

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

small nit: MFEs are utilizing and the header/footer ...

☝🏽 and looks a bit out of place in the sentence. This can be broken down into two sentences.

Copy link
Collaborator

@arbrandes arbrandes left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! I defer to @DawoudSheraz's comments, otherwise.

@DawoudSheraz DawoudSheraz self-requested a review May 7, 2024 09:07
README.rst Outdated
6. Install the customized header/footer components into your MFEs. This will resolve any npm dependency conflict issues.
7. All the steps outlined above need to be followed for the footer as well, if you have followed them for the header or vice versa.

.. image:: https://raw.githubusercontent.com/overhangio/tutor-mfe/master/screenshots/npm-conflict-deps.png
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: instead of screenshots, let's name the directory media, content, or something along these lines. screenshots is a very specific term.

@hinakhadim hinakhadim force-pushed the hina/update-troubleshooting branch from a9ba1da to b72a6b5 Compare May 13, 2024 05:56
@DawoudSheraz
Copy link
Contributor

@hinakhadim does this require further action? Thanks

@hinakhadim
Copy link
Contributor Author

Thanks for asking. I have made all improvements and no further action is reqiured.

@DawoudSheraz DawoudSheraz requested a review from arbrandes May 21, 2024 06:14
@DawoudSheraz
Copy link
Contributor

@arbrandes Hi, can you take another look at this? Thanks

@DawoudSheraz DawoudSheraz merged commit 15a0c6b into overhangio:master Jun 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants