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

All modules must be dependencies of primer-css... But should they? #236

Closed
shawnbot opened this issue Jun 23, 2017 · 3 comments
Closed

All modules must be dependencies of primer-css... But should they? #236

shawnbot opened this issue Jun 23, 2017 · 3 comments

Comments

@shawnbot
Copy link
Contributor

One thing that @broccolini and I had to do to get the primer-css module building successfully in the new monorepo structure (see #230) was to manually add all of the submodules to the dependencies in primer-css/package.json. Here's an example of why:

  1. primer-css imports primer-core, which can be resolved by Sass in primer-css/node_modules because it's a dependency.
  2. primer-core imports primer-support, which doesn't exist in primer-css/node_modules unless primer-css also depends on primer-support.

In other words, generally speaking: Every primer module currently must list all of its dependencies' dependencies (and so on) in order to build successfully with primer-module-build. This is... not ideal. It's not super brittle because we've already listed all of the modules — but the next time that we do add a module, we need to be sure to add it as a dependent of primer-css in addition to any other modules that use it.

We could potentially get around this by changing the way that primer-module-build resolves imports, or look at a tool like Eyeglass that does this already. Whether we decide to fix it or not, it's something to be aware of. 🚧

@shawnbot
Copy link
Contributor Author

I think that as of #475 we can update the primer/package.json to only depend on the meta-packages. This is something we should test.

@shawnbot shawnbot self-assigned this Jun 18, 2018
@shawnbot shawnbot removed this from 10.6.1 To Do in 📦 Primer CSS release tracking Jun 18, 2018
@shawnbot
Copy link
Contributor Author

FYI, I've tested this and everything builds without issue when all but the meta-packages are removed as dependencies from primer. Maybe something to save for v11 though?

@shawnbot
Copy link
Contributor Author

This is no longer an issue as of #671! 🎉

📦 Primer CSS release tracking automation moved this from v12.0.0 Release to 💜 Done Feb 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

1 participant