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

feat: tailwind configuration works in mono repos #2847

Open
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

mnahkies
Copy link
Contributor

Description

Previously the tailwind configuration only worked if the directory structure looked like:

/root
├── package.json
├── pages
├── evidence.plugins.yaml
├── ...
├── .evidence
│   └── template
│       ├── tailwind.config.cjs
│       └── node_modules # ./node_modules
│           └── some-module
│               └── dist/**/*.{html,js,svelte,ts,md}  
└── node_modules # ../../node_modules
    └── some-module
        └── dist/**/*.{html,js,svelte,ts,md}  

This PR aims to replace the assumptions on possible node_modules locations with following the same package resolution algorithm used elsewhere in the sdk, meaning that a hoisted monorepo layout like this will now work:

/root
├── package.json
├── node_modules
│   └── some-module # will now work
│       └── dist/**/*.{html,js,svelte,ts,md}  
└── packages
    └── my-evidence-proj
        ├── package.json
        ├── node_modules
        │   └── some-other-module # will still work
        │       └── dist/**/*.{html,js,svelte,ts,md}
        ├── pages
        ├── evidence.plugins.yaml
        ├── ...
        └── .evidence
            ├── template
            └── tailwind.config.cjs

There remains an assumption/convention that packages must structure their artifacts into a /dist/**/*.{html,js,svelte,ts,md} path, but that seems fine to leave alone.

Checklist

  • For UI or styling changes, I have added a screenshot or gif showing before & after
  • I have added a changeset
  • I have added to the docs where applicable
  • I have added to the VS Code extension where applicable

@mnahkies mnahkies requested a deployment to Approval required to run action on external PR November 23, 2024 10:02 — with GitHub Actions Waiting
@mnahkies mnahkies requested a deployment to Approval required to run action on external PR November 23, 2024 10:02 — with GitHub Actions Waiting
// note: this will return the directory of the package entrypoint, eg: `some-package/dist/index.js`
// so we need to walk up the tree until we find a `package.json` file to discover the root.
let pluginPackageDirectory = path.dirname(createRequire(import.meta.url).resolve(packageName));
let pluginPackageDirContents = fs.readdirSync(pluginPackageDirectory);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using sync filesystem access, as calling from a synchronous context in the tailwind config

@@ -78,6 +78,12 @@
"import": "./src/plugins/index.js",
"require": null,
"types": "./src/plugins/index.js"
},
"./plugins/discoverPluginPackageRootPathSync": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using a dedicated export, as needs to be required from a commonjs context, and esm only modules are included in the main ./plugins export

@ItsMeBrianD
Copy link
Member

A few notes on this one;

  1. I think we can rather easily convert tailwind configuration from .cjs to .mjs, should help us avoid needing to add another export path to the sdk (we have our ideal end state for the package documented in the monorepo roadmap doc
  2. This is not working for me when I run it locally in our monorepo, but the next branch does still work - there might be a regression here

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.

2 participants