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

Constructible Stylesheet Support #1346

Closed
mattgoldspink opened this issue Jun 7, 2019 · 23 comments
Closed

Constructible Stylesheet Support #1346

mattgoldspink opened this issue Jun 7, 2019 · 23 comments
Assignees
Labels
enhancement need consensus issues marked as such need some discussion next-release open source

Comments

@mattgoldspink
Copy link

Is your feature request related to a problem? Please describe.

We have a lot of existing stylesheets and design ourselves and customers use. Take SLDS for example, we use the slds-p-* and slds-m-* classes very heavily. Trying to inline these into each component to reproduce the existing structure and layout we have is a painful and slow process. In addition if not done correctly we get out of sync styles - for example if we don't turn all the sizing into variables and hardcode the values as verbatim rems we have to update the rems in all of the places they're used.

Describe the solution you'd like

Ideally we'd include a stylesheet in the page and it's styles would cascade down the Shadow DOM tree, but that also kind of defeats the purpose of Shadow DOM. Constructible Stylesheets seems like the current proposal that people are looking to standardize on:

  • Chrome has native support since v73.0
  • Microsoft Edge (latest chromium based Mac build) seems to support it already too
  • Firefox seems to be considering it Constructable Stylesheets mozilla/standards-positions#103
  • Apple's have reservations about the api, but from the comments on the above firefox ticket they think the general idea is good.

A polyfill exists (used and supported in Polymer): https://github.com/calebdwilliams/construct-style-sheets

Describe alternatives you've considered

Alternatives:

  • Disable Shadow DOM and use just synthetic DOM -this used to work in the earlier 0.36.x release but doesn't look like this is possible in 1.0.0 any more
  • Move all CSS into the components - I've started doing this on our existing project with slds and... it just feels wrong! SLDS was built to enable quick styling using an existing set of designs and patterns, but having to find the rules in SLDS and copy/paste them into each component is a slow process and as said before easily leading to errors.
@diervo
Copy link
Contributor

diervo commented Jun 7, 2019

@mattgoldspink we are in the process of enabling CSS only components, which means you can import them on your css as @imports() as they were constructible stylesheets (note that this must remain an implementation detail so we can support synthetic shadow in platform).

In the last TPAC meeting it was good consensus about constructible stylesheets (although some issue to sort with Apple) and we are just waiting on resolving some nuances with regards to CSS modules to allow for both programatic and declarative.

So long story short, we will probably be trying to use constructible stylesheets soon when available and falling back into our compilation when not available.

Will keep this issue for tracking purposes.

@diervo diervo self-assigned this Jun 7, 2019
@caridy caridy added enhancement need consensus issues marked as such need some discussion next-release labels Jun 7, 2019
@mattgoldspink
Copy link
Author

That’s awesome -I’ll shut up and let you guys get on with the good stuff! Thanks @diervo

@rtm
Copy link

rtm commented Jul 31, 2019

@diervo I'm curious why you would structure a solution as "CSS-only components"--presumably meaning they would live in their own directory, and have meta data as well, and be imported via the non-standard c/fooStyle style. Why not just let people import CSS files somewhere in their tree?

I love constructible stylesheets as a concept and an optimization, but I don't need them--I just need a shared set of styles.

My current workaround is a preprocessor which essentially replicates the way @import should work. We really shouldn't have to be doing things like this. You already use postCSS (I think), which already knows how to do imports--can't they be made to "just work"?

@diervo
Copy link
Contributor

diervo commented Jul 31, 2019

In LWC we are opinionated on the default folder structure for modules. CSS only components is so you can trivially (and locally) create your own CSS shareable modules within the standard LWC structure.

However, if you really want to, you can certainly create CSS anywhere (even from npm packages), but you have to tell the module system how to find your modules. If you are interested, take a look at this: #1414

@caridy
Copy link
Contributor

caridy commented Aug 1, 2019

Notice that the CSS only modules are only importable right now from a css file.. until the spec for css modules is cleared up, then we will support importing them in JS to be added to the adoptedStyleSheet, it will take a while.

@rtm
Copy link

rtm commented Aug 1, 2019

@caridy So CSS-only modules work now, or are they "in the process of being enabled"? I don't see anything about this in the docs though.

@rtm
Copy link

rtm commented Aug 1, 2019

@diervo Thanks for your reply. Don't mean to take this off-topic, but I am all in favor of opinionated solutions but when I understand the reasons for the opinion, which I don't here. Also, this seems to go a bit beyond being opinionated to changing well-known, industry-wide semantics for module resolution in a way which makes development less convenient. If this had something to do with the SF security and management model, I could at least understand, but now LWCs are being pitched as a solution for non-SF applications, so that can't be the problem...or what am I missing?

With regard to the module/directory structure, the feature I am now working on has a half-dozen interrelated components that I want to group into a directory such as force-app/main/default/lwc/my-feature, which would in turn contain the directories for individual components, but when I do this it can't seem to find my components and fails when I try to deploy them. Is that structure not supported, or am I just missing something?

@caridy
Copy link
Contributor

caridy commented Aug 1, 2019

@caridy So CSS-only modules work now, or are they "in the process of being enabled"? I don't see anything about this in the docs though.

It is available in 1.0.3 from NPM (soon to be tagged latest). In platform, it will take longer, platform is usually behind OSS for few months.

@caridy
Copy link
Contributor

caridy commented Aug 1, 2019

@rtm the folder structure is one of those things that we struggled a lot when designing LWC, mostly because of the constrains of the existing platform (components stored in DB). In fact, that folder structure that you are attempting to use was almost identical to the first implementation in LWC, which we ended up changing to stay within the boundaries of the platform to support easy portability from and into the platform.

We ended up providing an implicit (opinionated) folder structure for LWC modules/bundles, with the ability to define an explicit mapping if you want to do your own thing (which is the map that @diervo just mentioned).

On top of all that... there is a bigger industry-wide effort going on to define how web components should be shared (you can just look at twitter from last night were we were discussing that), once we solve that, we will probably adjust to match that somehow.

@rmartinezffdc
Copy link

Hi all,

We are currently experiencing some issues in the integration of the SLDS css in LWC OSS because of he scoped CSS nature of the shadow HTML. We would need to check the possibility of disabling Shadow DOM.

As workaround we are also using the constructor of every element to include the SLDS css file by modifying directly the DOM of the template, which is obviously not an optimal solution. Also it enforced us to implement a way to wait until the CSS file is totally loaded hiding the content with a spinner in the meantime.

Is there any native way to include an additional CSS file? It would also help if the render hook would wait to actually render until every CSS file is totally loaded. As other possible solution, it would be great if Salesforce would offer a module able to process the complete SLDS css and include in every LWC OSS element only those classes which are really needed.

Thanks!

@muenzpraeger
Copy link

If you don’t want to use Shadow DOM you can use the synthetic version (which is the one that runs on the Salesforce platform).

Just add import '@lwc/synthetic-shadow'; to your index.js file.

@caridy
Copy link
Contributor

caridy commented Sep 16, 2019

@rmartinezffdc we will be working on a referral implementation this week for folks using SLDS in the open, I will make sure that we link the solution to this issue.

@isalew
Copy link

isalew commented Feb 20, 2020

I found this page about shared style sheets in the documentation today, but when I tried using it, was unable to deploy my css-only library. Is this a working feature now?

@diervo
Copy link
Contributor

diervo commented Feb 20, 2020

Did if failed on open source or in platform? Can you share more info about stack traces or some other data point?

@isalew
Copy link

isalew commented Feb 21, 2020

@diervo it was in platform. I attempted an sfdx force:source:deploy command to a dev org and received the following error:

sfdx.log

[
   {
      "name": "sfdx",
      "hostname": "****",
      "pid": "****",
      "log": "SourceDeployCommand",
      "level": 30,
      "msg": [
         {
            "error": "Cannot find Lightning Component Bundle lib.",
            "fullName": "lib/lib.js",
            "type": "LightningComponentBundle",
            "filePath": "force-app/main/default/lwc/lib/lib.css",
            "problemType": "Error",
            "height": 1
         }
      ],
      "time": "2020-02-21T00:26:26.232Z",
      "v": 0
   },
   {
      "name": "sfdx",
      "hostname": "****",
      "pid": "****",
      "log": "SourceDeployCommand",
      "level": 50,
      "msg": [
         "\u001b[1mERROR running force:source:deploy: \u001b[22m",
         "\u001b[31mDeploy failed.\u001b[39m"
      ],
      "time": "2020-02-21T00:26:26.236Z",
      "v": 0
   }
]

lib.css

p.shared {
    background-color: green;
}

@diervo
Copy link
Contributor

diervo commented Feb 21, 2020

@Gr8Gatsby Wasn't this a bug we fixed?

@Gr8Gatsby
Copy link

@diervo @isalew Yes this will be resolved in the upcoming release. When Summer rolls out you will be able to save .css only modules

@celliott181
Copy link

@diervo @Gr8Gatsby A coworker and I ran into some undocumented behavior deploying a CSS-only LWC that's a dependency of another LWC wrapped in an Aura component.

Using sfdx force:source:deploy returns an error saying the component doesn't exist. It works fine if you deploy with the CSS LWC import removed, and then deploy again after the Aura component has made it into the org. Unfortunately you need to repeat this process to deploy the Aura component again, e.g. into a new scratch org or after changes. Is this expected?

Here's a slightly redacted log illustrating the issue.

  "status": 1,
  "result": [
    {
      "error": "No MODULE named markup://c:cssLibrary found : [markup://c:UtilityBarAuraApp, markup://c:utilityBarApp]",
      "fullName": "UtilityBarAuraApp/UtilityBarAuraApp.cmp",
      "type": "AuraDefinitionBundle",
      "filePath": "force-app/main/default/aura/UtilityBarAuraApp/UtilityBarAuraApp.cmp",
      "problemType": "Error"
    }
  ],
  "name": "DeployFailed",
  "message": "Push failed.",
  "exitCode": 1,
  "actions": [],
  "commandName": "SourcePushCommand",
  "data": [
    {
      "error": "No MODULE named markup://c:cssLibrary found : [markup://c:UtilityBarAuraApp, markup://c:utilityBarApp]",
      "fullName": "UtilityBarAuraApp/UtilityBarAuraApp.cmp",
      "type": "AuraDefinitionBundle",
      "filePath": "force-app/main/default/aura/UtilityBarAuraApp/UtilityBarAuraApp.cmp",
      "problemType": "Error"
    }
  ],
  "stack": "DeployFailed: Push failed.\n    at ~/.local/share/sfdx/node_modules/salesforce-alm/dist/lib/source/sourceApiCommand.js:70:31\n    at async SourcePushCommand.execLegacyCommand (~/.local/share/sfdx/node_modules/salesforce-alm/dist/ToolbeltCommand.js:148:29)\n    at async SourcePushCommand.run (~/.local/share/sfdx/node_modules/salesforce-alm/dist/commands/force/source/push.js:29:16)\n    at async SourcePushCommand._run (~/.local/share/sfdx/node_modules/salesforce-alm/node_modules/@salesforce/command/lib/sfdxCommand.js:93:40)\n    at async Config.runCommand (~/.local/share/sfdx/client/7.74.1-32db2396ed/node_modules/@oclif/config/lib/config.js:173:24)\n    at async Main.run (~/.local/share/sfdx/client/7.74.1-32db2396ed/node_modules/@oclif/command/lib/main.js:27:9)\n    at async Main._run (~/.local/share/sfdx/client/7.74.1-32db2396ed/node_modules/@oclif/command/lib/command.js:43:20)\n    at async Object.run (~/.local/share/sfdx/client/7.74.1-32db2396ed/dist/cli.js:32:20)",
  "warnings": []
}

@caridy
Copy link
Contributor

caridy commented Oct 2, 2020

@gelliott181 it is hard to tell without the actual code of the importer.

@celliott181
Copy link

@caridy Ok, I'll find some time to try and reproduce with a more generic example for you guys to take a look at.

@nolanlawson
Copy link
Collaborator

I'd like to better understand what this issue is asking for precisely. As noted above, we already have a way to import shared CSS into components:

/* component.css */
@import './shared.css';

The fact that it currently doesn't use constructed stylesheets is an implementation detail. This pattern can work with and without constructable stylesheets (see #2460), and in fact it will have to, in order to support browsers that don't support constructable stylesheets.

Constructable stylesheets are a perf optimization, but from what I've seen, it's not a big enough perf difference to be an enabling factor. Is this issue asking for something else, or is @import a sufficient solution?

@mgoldspink-salesforce
Copy link

HI @nolanlawson, I actually think the solution we need is now available as Light DOM. That would solve most of the issues we were facing originally.

@nolanlawson
Copy link
Collaborator

@mgoldspink-salesforce That's great to hear! I'll close this issue then. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement need consensus issues marked as such need some discussion next-release open source
Projects
None yet
Development

No branches or pull requests