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

Add artifactDirectory option support to embedded-document-artefact-loader/ts-jest #414

Merged

Conversation

mwangi-njuguna
Copy link
Contributor

Adds support for explicitly passing artifactDirectory option in jest-config when using artifactDirectory in relay config.

@alloy
Copy link
Member

alloy commented Apr 29, 2024

@mwangi-njuguna Can you please elaborate on where/why you are using the artifactDirectory config? I understand what the config does in Relay, I contributed it, but I need to understand why we would need it inside MS?

@Markionium
Copy link
Member

@alloy the reason we're currently using it is because of the following:

  • Some of the Nova components inside 1JS use different compilers (not all of them are on the Relay one yet). Which means that the generated files inside those packages are not Relay compatible.
  • The other, probably primary, reason is to prevent writing files into other packages in the mono repo (This is a thing we try to stick to so the engineering system folks can optimise build times).

e.g. If my package is app-package and I run the relay compiler there, I don't want relay-compiler to write files into a dependency-package. So what we've done so is to use artifactDirectory to generate all the files for an app in 1JS inside the apps package and have webpack pick up the files from there using either @graphitation/embedded-document-artefact-loader/webpack or @swc/plugin-relay.

This has been working great so far for the app, but right now we're trying to fix up our tests and thus the reason for this PR :)

As an FYI, this isn't for Jana, we're using this for Org Explorer.

@alloy
Copy link
Member

alloy commented May 2, 2024

I'm worried about runtime values being imported from a centralised artefact directory with a manual written import, which then wouldn't work in a host app that doesn't use a centralised artefact directory.. I guess it should normally only be typings, so I'll approve this, but let's be mindful of that.

@alloy alloy merged commit 8e1f555 into microsoft:main May 2, 2024
2 checks passed
@mwangi-njuguna mwangi-njuguna deleted the add-artifactDirectory-to-jest-config branch May 2, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants