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

enhance(bcd): Remove use of @mdn/bcd-utils-api in testing #10186

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Dec 14, 2023

Note to reviewers: because merge conflicts will occur any time that BCD updates, I will fix them right after this is approved.

Summary

This PR removes the use of @mdn/bcd-utils-api during testing (and potentially, could also be removed in production), allowing the use of BCD directly.

Note: requires BCD v5.5.0+.

Problem

In the past, we used to be able to use npx install-local to import BCD for testing purposes. Unfortunately, with the introduction of the @mdn/bcd-utils-api package, this is no longer possible, making local testing virtually impossible.

Solution

Instead of using the middleware, this modifies the code and routers to use BCD directly once again. Both of the new properties that @mdn/bcd-utils-api adds can be bypassed:


Testing the changes

To test the change, I did the following:

  • Checked out a copy of BCD with the corresponding PR merged (the latest main)
  • Ran npm run build in BCD
  • In Yari, ran npx install-local ../browser-compat-data/build
    • (Actually, I just copied the files manually; there seems to be some NPM/Yarn conflicts that install-local catches)
  • Ran the development server
  • Checked a page with a compatibility table that would have a version_removed and compared it to the deployed version of MDN for any differences
    • Left side is the public version, right side is the local build
image

@queengooborg queengooborg requested review from mdn-bot and a team as code owners December 14, 2023 22:34
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Dec 14, 2023
@queengooborg queengooborg changed the title Remove use of @mdn/bcd-utils-api in testing enhance(bcd): Remove use of @mdn/bcd-utils-api in testing Dec 15, 2023
@github-actions github-actions bot added merge conflicts 🚧 Please rebase onto or merge the latest main. labels Dec 18, 2023
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

1 similar comment
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Dec 18, 2023
@github-actions github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Dec 21, 2023
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added merge conflicts 🚧 Please rebase onto or merge the latest main. and removed merge conflicts 🚧 Please rebase onto or merge the latest main. labels Dec 21, 2023
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added merge conflicts 🚧 Please rebase onto or merge the latest main. and removed merge conflicts 🚧 Please rebase onto or merge the latest main. idle labels Mar 20, 2024
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@caugner
Copy link
Contributor

caugner commented Apr 4, 2024

Unfortunately, with the introduction of the @mdn/bcd-utils-api package, this is no longer possible, making local testing virtually impossible.

@queengooborg Can you explain why npx install-local no longer works since the introduction of @mdn/bcd-utils-api? Since that package uses @mdn/browser-compat-data, our expectation is that this should still work!?

@queengooborg
Copy link
Collaborator Author

Well, there's one big reason why using BCD directly doesn't work: Yari depended on custom properties provided by BCD-utils transforms!

@caugner
Copy link
Contributor

caugner commented Apr 4, 2024

Well, there's one big reason why using BCD directly doesn't work: Yari depended on custom properties provided by BCD-utils transforms!

I'm not sure I understand. Currently we fetch the data via getBCDDataForPath() from @mdn/bcd-utils-api, so why doesn't that transform the data from your locally installed @mdn/browser-compat-data?

@caugner caugner added the awaiting response Awaiting for author to address review/feedback label Apr 17, 2024
@github-actions github-actions bot added the idle label May 17, 2024
@github-actions github-actions bot removed the idle label Aug 16, 2024
@github-actions github-actions bot added the idle label Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Awaiting for author to address review/feedback dependencies Pull requests that update a dependency file idle merge conflicts 🚧 Please rebase onto or merge the latest main.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants