akanix42 / meteor-css-modules

MIT License
92 stars 20 forks source link

Question about sharing styles between app and package #129

Open arggh opened 5 years ago

arggh commented 5 years ago

First and foremost, I'd like to thank you for an amazing package. We've been using it in production for some years already and it's just pure awesome.

Now, I've come by a situation where I thought some guidance might help greatly.

In our app, we've applied this type of pattern generously, where...

This has worked wonderfully and has allowed me to define the basic bits and pieces of our styles in a reusable fashion. This way, if I have 5 different components that have some sort of inputs, all of the use compose to derive their basic apperance from those shared stylsheets. All good so far.

Now the problem:

I'm in the process of moving all our common components, like modal, title, button etc. to a Meteor package.

Is there an existing way of still sharing those common stylesheets across the app / package boundary, so that they could be used in both places?

My initial plan of action was to put the stylesheets in a dedicated directory outside of both app and package folders and then create symbolic links to both the app and package to be consumed by CSS Modules (we have a monorepo structure, but Git submodules could achieve the same effect).

Is there a better way, perhaps?

I already learned about sharing config between the app and package from this issue, but I didn't find anything about sharing stylesheets in any of the existing issues.

Once again, thanks for this package!

akanix42 commented 5 years ago

It's been a very long time since I've used it, but the syntax (handled here in the code) should be {}/path/to/file.css for referencing a file in the base app from a package, and {your:package}/path/to/file.css for referencing a file in a package from the base app or another package. It will only work for local packages, not Atmosphere packages.

Examples for packages:

arggh commented 5 years ago

I tried to give this a try:

Reference the package's stylesheet with the provided syntax in my app's stylesheet with

.title {
  composes header from '{arggh:ui}/styles/typography.m.css`;
}

and I have a package, arggh:ui, which contains /styles/typography.m.css.

but for some reason, I get build errors like so:

Unable to resolve some modules:
"{arggh:ui}/styles/typography.m.css" in /Users/arggh/Development/myapp/client/modules/account/signup/signup.m.css.js
(web.browser.legacy)`
Unable to resolve some modules:
"{arggh:ui}/styles/buttons.m.css" in /Users/arggh/Development/myapp/client/modules/account/signup/signup.m.css.js
(web.browser.legacy)`

And in the browser:

Uncaught Error: Cannot find module '{arggh:ui}/styles/typography.m.css'

I tried to:

a) import those stylesheets directly in my app via import 'meteor/arggh:ui/styles/typography.m.css etc. b) import those stylesheets in the package's mainModule and then just import the package import 'meteor/arggh:ui';

...but it didn't do any good. I wonder what I might be doing wrong?

arggh commented 5 years ago

Trying to replace references of {arggh:ui} with just {ui} results in these errors:

While processing files with nathantreid:css-modules (for target web.browser.legacy):
   packages/caching-compiler/multi-file-caching-compiler.js:137:21: Unknown absolute import path
   /Users/arggh/Development/myapp/packages/ui/styles/layout.m.css
   at referencedImportPaths.forEach.path (packages/caching-compiler/multi-file-caching-compiler.js:137:21)
   at Array.forEach (<anonymous>)
   at getResult (packages/caching-compiler/multi-file-caching-compiler.js:135:33)
   at inputFiles.forEach.inputFile (packages/caching-compiler/multi-file-caching-compiler.js:167:24)
   at Array.forEach (<anonymous>)
   at Promise.asyncApply (packages/caching-compiler/multi-file-caching-compiler.js:93:16)

However, this/Users/arggh/Development/myapp/packages/ui/styles/layout.m.csspath is very much real and exists.

arggh commented 5 years ago

Using the symlink strategy it seems I managed to get this style sharing pattern to work, though I would have much preferred the syntax you proposed for being more concise!

akanix42 commented 5 years ago

Glad to hear that worked, but please give nathantreid:css-modules@4.1.0-beta.1 a try! I've added support for the meteor/ syntax, so you can do the following:

  1. In client/main.m.css:
    .title {
    composes: header from 'meteor/arggh:ui/styles/typography.m.css';
    }
  2. In packages/arggh_ui/styles/typography.m.css:
    .header {
    font-size: 50px;
    }
  3. In packages/arggh_ui/package.js:

    Package.onUse(function(api) {
    api.versionsFrom('1.8.0.2');
    api.use('ecmascript');
    api.use('nathantreid:css-modules@4.1.0-beta.1');
    
    api.addFiles('styles/typography.m.css', 'client');
    });

Sample project here: https://github.com/nathantreid/css-modules-129

arggh commented 5 years ago

Actually I take my word back, composes wasn't working in the package with symlinks. For example, in the package I had a file /button/button.m.css, which would contain this:

.roundedButton {
   composes: rounded from '/styles/buttonStyles.m.css`;
}

...those composed styles did not get included.

But with the 4.1.0-beta.1, it works!

I did as you instructed, but with one "odd" addition:

I had to refer to the shared stylesheets with the meteor/arggh:ui/ prefix even inside the package arggh:ui. Is that as you intended? I expected direct path reference to work there.

Anyway, this is really great, thanks!

akanix42 commented 5 years ago

Hmm, direct path reference should work inside the packages - I'll have to take a look. Thanks for the feedback!

arggh commented 5 years ago

The console was bombarded by hundreds of missing file errors on build when trying to reference those shared stylesheets directly, but after prepeding meteor/arggh:ui to the file paths (inside arggh:ui), everything just worked.

arggh commented 5 years ago

Though, I'm having other issues with the Meteor bundler at the same time, I have no idea if related at all: https://github.com/meteor/meteor/issues/10496

They don't seem to be connected from my perspective, but I thought I should mention.

arggh commented 5 years ago

@nathantreid I've been running the beta with no issues so far.