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: container query support via css-tree extension #8275

Merged
merged 8 commits into from
Mar 27, 2023
Merged

feat: container query support via css-tree extension #8275

merged 8 commits into from
Mar 27, 2023

Conversation

typhonrt
Copy link
Contributor

@typhonrt typhonrt commented Feb 10, 2023

Closes #6969. As discussed there, container query support is quite useful to add to Svelte as it is now broadly available with Firefox releasing support imminently w/ FF v110 this upcoming week (~Feb 14th). Chrome has had support since ~Aug '22. The central issue is that css-tree which is a dependency for CSS AST parsing is significantly lagging behind on adding more recent features such as container query support. Ample time has been given to the maintainer to update css-tree and I do have every confidence that in time css-tree will receive a new major version with all sorts of modern CSS syntax supported including container queries. This PR provides an interim solution for what Svelte needs to support container queries now.

This PR broadly helps the entire Svelte ecosystem including SvelteKit.

Changes

src/compiler/parse/read/style.ts: Switch to locally forked css-tree w/ CQ support.

src/compiler/parse/read/css-tree-cq/css_tree_parse.ts: fork css-tree w/ local extension.

src/compiler/parse/read/css-tree-cq/node: css-tree new CQ nodes.

src/compiler/compile/css/Stylesheet.ts: Trivial one line change to enable CQ support in Svelte.

test/css/samples/container-query: Parsing test case.

Impact

No downsides that I'm aware of regarding these changes. Simply that Svelte can now ship 3.56.0 w/ CQ support. I have thoroughly tested the code and added an additional test to the svelte repo. Because I'm using the fork capability of css-tree to extend it the css-tree version can keep being bumped if new versions come out without CQ support. However, when CQ support is added my local extensions can be removed and an easy swap back to the official css-tree parse capability is trivial.

Testing

A new test was added in the css section container-query that runs through several common usage permutations. Just like the media-query test it contains positive tests that ensure parsing.

css-tree Extension

The bulk of this PR is local extension of css-tree using the fork capability of css-tree to add new parsing nodes that implement the parsing of @container. The benefit of taking this approach is that the temporary modifications necessary to extend css-tree are committed to the Svelte repo and a separate hard fork of css-tree isn't necessary. The css-tree version can continue to be bumped as necessary and when css-tree supports container queries the local extension can be removed.

It is unknown when css-tree will be updated to support container queries and other more recent style options like media range query syntax. However, when css-tree does offer support a one line change and removal of the extensions provided in this PR is trivial.

I would be glad to discuss the css-tree specific parsing code added. I certainly recognize that an internal knowledge of css-tree is probably not widely held by most of the Svelte maintainers. My extension follows current patterns used in css-tree itself.

Instead of duplicating all conversation from #6969 Please refer to that issue for pertinent prior discussion. As indicated in the last few comments there (#6969 (comment)) I have released a temporary version of Svelte 3.55.1 on NPM that can be used to test these changes now in live environments. Two other community members responded that these changes work as intended including a SvelteKit trial.

Tagging: @tanhauhau (you created the css-tree issue in response to the initial triage of #6969) and @dummdidumm. Please include any other relevant maintainers for review.

@vercel
Copy link

vercel bot commented Feb 10, 2023

@typhonrt is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@benmccann benmccann changed the title Feat: Added container query support via css-tree extension. Closes #6969. feat: container query support via css-tree extension Feb 14, 2023
@Snugug
Copy link

Snugug commented Feb 16, 2023

Thanks for your work on this @typhonrt! I'm currently testing your fork on a fairly complex component I've written using Sass (style lang="scss"). The parser you've written for CQs is unfortunately choking on Sass interpolation (@container (min-width: #{$swap}) for Sass. The error is:

 Number, dimension, ratio or identifier is expected

Sass does support this already (I'm converting a component written in Astro to Svelte here, with Sass already in place), so I think this generally should be supported here.

@dummdidumm
Copy link
Member

That #{$swap}) thing is sass-specific syntax, right? If so, this needs to run through a preprocessor first (like svelte-preprocess). Svelte only deals with vanilla CSS.

@Snugug
Copy link

Snugug commented Feb 16, 2023

That #{$swap}) thing is sass-specific syntax, right? If so, this needs to run through a preprocessor first (like svelte-preprocess). Svelte only deals with vanilla CSS.

It is. I have it running through the Sass preprocessor. Sass on its own supports this syntax., and does correctly compile it to a number. The error I'm getting in there terminal points to my pre-processed CSS, leading me to assume the parser hits my styling before Sass does.

It also fails when using calc in min-width, which is not a Sass-specific syntax, so it's not confined to Sass syntax only.

EDIT: Interestingly enough, the parse error in my browser and in my terminal are different; in my browser it points to the Sass interpolation, but in my terminal it specifically points to calc only; maybe this is only an issue with using calc, which is valid syntax for container queries

@typhonrt
Copy link
Contributor Author

typhonrt commented Feb 16, 2023

It is. I have it running through the Sass preprocessor. Sass on its own supports this syntax. If it was simply a matter of Sass supporting this syntax, I'd assume that we wouldn't need this PR and we could just throw lang="scss" on everything and it work. That's doesn't appear to be the case; this parser runs before it gets to preprocessing (can be tested in current Svelte by container queries not working at all without this update, even if using Sass). The CSS processor does support some Sass-level syntax, specifically it doesn't choke in other places where I use Sass-specific syntax, including Sass-style nesting.

I also use Sass in my dev efforts, but have svelte-preprocess hooked up and everything works. It sounds like you are setting up another Sass preprocessor in your build setup?

Test case:

   $swap: 50px;

   @container (min-width: #{$swap}) {
      main {
         background: blue;
      }
   }

Generates:

@container (min-width: 50px) {
   main.svelte-1c1au4r.svelte-1c1au4r {
      background:blue
   }
}

@Snugug
Copy link

Snugug commented Feb 16, 2023

Yah, sorry for the churn here, I think I was reporting the right error on the wrong place in my source code; getting conflicting items from my build tool. I think it's choking on calc, not Sass interpolation

@typhonrt
Copy link
Contributor Author

typhonrt commented Feb 16, 2023

Yah, sorry for the churn here, I think I was reporting the right error on the wrong place in my source code; getting conflicting items from my build tool. I think it's choking on calc, not Sass interpolation

Of interest in general is that css-tree doesn't seem to support that syntax either for media queries; IE using calc, so it is not something specific per se to this PR.

Test case:

   $swap: 50px;

   @media(min-width: calc(#{$swap} + 1)) {
      main {
         background: blue;
      }
   }

Fails to parse:

[vite-plugin-svelte] /view/hello/HelloFoundryAppShell.svelte:73:38 Number, dimension, ratio or identifier is expected
 71 |  </ApplicationShell>
 72 |  
 73 |  <style lang="scss">@media (min-width: calc(50px + 1)) {
                                             ^
 74 |    main {
 75 |      background: blue;

However if you use calculation built into Sass it works:

   @media(min-width: #{$swap + 1}) {
      main {
         background: blue;
      }
   }

Results in min-width: 51px

In general and quite likely why it will take a bit for css-tree to catch up is that there is a fair amount of new syntax to support thoroughly. My general understanding is that calc in queries has been from what I can tell with a cursory search supported in browsers around ~2018.

@Snugug
Copy link

Snugug commented Feb 16, 2023

So for my usecase, I need actual calc support (calc(100vw - 1rem)) so I can't use Sass's built-in support. Thanks for the investigation. Wonder if there's another option that's more up-to-date.

@typhonrt
Copy link
Contributor Author

typhonrt commented Feb 16, 2023

So for my usecase, I need actual calc support (calc(100vw - 1rem)) so I can't use Sass's built-in support. Thanks for the investigation. Wonder if there's another option that's more up-to-date.

I have looked at other CSS parsers and I don't believe there is an OSS one that actually handles modern query syntax presently. For instance spin up AST Explorer

You'll see that by default w/ current available versions of css-tree and postcss that the preludes are not parsed for either.

css-tree has a setting which Svelte enables to throw an error on parse failures.
https://github.com/sveltejs/svelte/blob/master/src/compiler/parse/read/style.ts#L34-L36

By defining onParseError when invoking css-tree parse and re-throwing any errors this causes the process to fail. Removing onParseError will have css-tree generate a raw node and no children array for the prelude that doesn't parse; this won't cause a parse build failure, but for instance will trip up minification due to this snippet for media queries. Technically Svelte doesn't use any of the at-rule prelude AST nodes as far as I'm aware and just the main block node for the CSS hash substitutions.

There also is an option in css-tree to skip parsing at-rule preludes, but that applies to all at-rules.

I don't think there is a clear cut OSS CSS AST parser to switch to... I wished it could be as easy to suggest a workaround by commenting out the onParseError config, but that has other downstream issues described above without further, but small modifications / hardening considerations. In general I'd gather the Svelte maintainers would prefer a robust parser w/ parse errors turned on by default.

The calc issue that you brought up is certainly pertinent, but quite likely can be filed as a separate issue to solve via waiting for css-tree to catch up or relaxing parsing constraints / don't throw on parse errors and hardening the small amount of follow on code that expects a parsed prelude for at-rules. Perhaps an option to the Svelte compiler could be added to relax CSS parsing w/ the extra mods to harden the small amount of follow on code putting the burden on the developer to write correct CSS / Sass styles when using the very latest syntax.

Definitely straying from the purpose of this PR though. It is good to realize that currently there are more cases of modern CSS syntax that trip up css-tree. Media query range syntax also is not supported by css-tree though I have support for it in @container with this PR.

@typhonrt
Copy link
Contributor Author

typhonrt commented Feb 17, 2023

So, I did do a bunch of research / investigation yesterday. Functions that return a single number / value are valid where a single value can be used, but it's non-obvious from the media query / container query specs along with various conflicting historical discussion around the web. For the MQ spec I linked to the section where this is implied:

Properties sometimes accept complex values, e.g., calculations that involve several other values. Media features only accept single values: one keyword, one number, etc.

There is no mention of this in the CQ spec. It would be nice if the specs and MDN docs for MQ / CQ gave examples of this to make it more obvious, but they don't. See calc() expressions in the MQ browser compatibility table showing wide browser support. The single value CSS functions that apply are: calc, min, max, clamp.

I found discussion for css-tree that is good to read and has links to the other relevant specs and StackOverflow discussion. The maintainer of css-tree also wasn't aware of this situation and I'm sure it will be addressed in the large queries update that hopefully drops in the future.

I don't see a problem in adding support for those 4 functions in the parsing code of this PR and I will work on implementation aspects this weekend and release another update to my temporary Svelte fork on NPM so folks can test it out. I will do a bit more research to verify if functions are only applicable to some MQ / CQ properties and not others.

Given communication and support from the Svelte maintainers I don't see a problem with submitting an additional PR that addresses media queries using the same fork solution to duplicate the existing css-tree MQ nodes and add media range syntax + CSS function support to media queries as well. This is relatively low hanging fruit since the extension process via this PR will already in place and will also close #5876.

@z-x
Copy link

z-x commented Feb 21, 2023

Much needed feature, thank you very much! I am planning to use container queries on production app and was surprised with the error message. I do hope this will be merged soon!

@vercel
Copy link

vercel bot commented Feb 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
svelte-dev-2 ❌ Failed (Inspect) Feb 22, 2023 at 0:46AM (UTC)

@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 3, 2023

So, I have added single value CSS function support to the parsing. The function expression (content inside parenthesis) is not parsed, but verification of calc, clamp, max, min is for function name. This allows you to write query rules like:

@container test-container (calc(400px + 1px) <= width < calc(500px + 1px)) {}

Waiting on maintainer review(s). This PR has been kept up to date w/ latest commits to Svelte as of this message.

@Snugug and others you can test out this functionality now as I have a temporary fork of Svelte w/ this PR available with this NPM package. You can load it in a Svelte project with the following in package.json:

"overrides": {
   "svelte": "npm:@typhonjs-svelte/svelte@3.55.1-cq2"
},

@typhonrt typhonrt marked this pull request as draft March 3, 2023 00:17
@typhonrt typhonrt marked this pull request as ready for review March 3, 2023 00:18
@z-x
Copy link

z-x commented Mar 6, 2023

If that's any help - I've tested it out on some basic container queries over the weekend on my project and everything seemed to work like it suppose to.

@Snugug
Copy link

Snugug commented Mar 6, 2023

@typhonrt I can confirm that with the new version, my weird calc based container query works. TYVM for your work on this!

@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 14, 2023

Yet another minor update.. The PR is up to date w/ mainline Svelte as of this message. A new external version of Svelte 3.56.0 now 3.57.0 w/ the container query PR applied is available here with this NPM Package. You can load it in a Svelte project with the following in package.json:

"overrides": {
   "svelte": "npm:@typhonjs-svelte/svelte@3.57.0-cq"
},

Still waiting on Svelte maintainers to take a look. I'm going to ping you, @baseballyama, as you added the blocked by upstream label several weeks ago. Could you comment on this perhaps as this PR "unblocks upstream". I'd definitely be glad to start a conversation about this PR.

@baseballyama
Copy link
Member

baseballyama commented Mar 17, 2023

Thank you for your effort for this!
And sorry for late reply! (I have a only very limited time for OSS activities😅)

Can you please explain us why we need to send a PR to svelte instead of css-tree?🙏

In my understanding is that the best way is to support @container in css-tree itself.
I think css-tree doesn't have a technical limitation. refer: csstree/csstree#174 (comment)
Just PR is needed.
So it's better to send a PR to css-tree instead of here.
We don't want to add a maintenance cost for this if we can.

@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 17, 2023

Can you please explain us why we need to send a PR to svelte instead of css-tree?🙏

In my understanding is that the best way is to support @container in css-tree itself. I think css-tree doesn't have a technical limitation. refer: csstree/csstree#174 (comment) Just PR is needed.

If you read the css-tree issue you linked you will see that I and Ben have already asked if any external help can be utilized with no response. As things go the queries refactor / rewrite to css-tree is not something that will receive an outside contribution. I have already tried to engage in this manner as it would be best.

The css-tree maintainer mentioned the exact solution that I applied to extend css-tree w/ container query support:
csstree/csstree#174 (comment)

There has been no communication from the maintainer since then and the queries branch stalled out December 15th. It is unknown when css-tree will be updated for the latest standards. All major browsers have CQ support shipped.

The reason to extend css-tree and commit to the Svelte repo is that the alternative is a hard fork of css-tree which is highly undesirable. The extension process doesn't prevent bumping the css-tree package if / when new releases come out. When a mainline release of css-tree supports container queries a one line switch back to the mainline version is easy. Hopefully css-tree gets the big queries update soon, but waiting without any known date of delivery is past reasonable expectations at this point.

This PR includes a test for the added CQ support:
https://github.com/typhonjs-svelte/svelte-cq-pr/tree/master/test/css/samples/container-query

I have been releasing a fork of Svelte w/ the CQ PR applied for a while now and folks are starting to use it, so it has been tested externally with Svelte and SvelteKit projects.

@baseballyama
Copy link
Member

Thank you for your explanation!

I have one more question.
I think this is an easy workaround but what do you think?
Is CSS scoping critical for also @container?

/** app.css */
@container (max-width: 600px) {
  .container p {
    color: red;
  }
}
<!-- App.svelte -->
<div class="container">
  <p>Some text here.</p>
</div>

<style>
  .container {
    container-type: size;
  }
</style>

@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 17, 2023

Thank you for your explanation!

I have one more question. I think this is an easy workaround but what do you think? Is CSS scoping critical for also @container?

It is you are shipping a component library / UI framework as I do. There are workarounds so to speak, but only reasonable for an end project to utilize and they are clunky at best. Being able to scope CQ CSS to components is very useful.

This PR more or less is the "least worst" interim solution until the mainline release of css-tree that supports CQ is available. css-tree is also behind on media queries level 4 spec such as range syntax and using single value functions in queries ('calc'). There is a separate issue that has been open for a while #5876 such that if this css-tree extension PR is accepted I'd also like to submit a separate PR that addresses media query level 4 support by the same extension mechanism. Both of these areas should be addressed in a single future css-tree release; just when is the big unknown.

I do understand the potential maintenance burden. I spent a fair amount of time to understand the internals of css-tree applicable to this effort and certainly this isn't widely known to other Svelte maintainers. All of the css-tree code itself is undocumented, so it can be a challenge to review it. I followed the same patterns and code style used internally to css-tree to create this PR. While I'm not a Svelte maintainer "yet" I am quite committed to the cause. If any problems arise I can absolutely help though things are well tested. The good thing about this PR / fix is that it is a one line change to reverse it when css-tree is updated w/ modern query support.

@baseballyama
Copy link
Member

As a Svelte user, I agree with you.
But probability, after releasing the PR, some bug reports will come and we need to fix them.
But now Svelte core team is focusing on Svelte4. So it's not the right time to spend resources on this.

So I want to wait @continer supports in css-tree side as much as possible.

Already you released a forked version of Svelte. So people who need to support @container query, can use your forked version.
If the volume of downloads is a lot, we can again consider merging this.

This is my personal opinion for now.

@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 17, 2023

This is my personal opinion for now.

I do appreciate you providing input as I'm trying to start a conversation and hopefully gain some sort of consensus and I do hope other Svelte maintainers can be a part of this discussion. I certainly will try to answer some of your thoughts diplomatically as possible. :)

But probability, after releasing the PR, some bug reports will come and we need to fix them.

It is 100% broken right now. Something that works and is well tested providing an interim solution is better than 100% broken. This PR has a full test case. That it could break is a hypothetical. If such a problem occurred I'd be more than happy to fix it.

But now Svelte core team is focusing on Svelte4. So it's not the right time to spend resources on this.

No resources for the core team need to be expended as this PR is a complete solution other than discussing the merit of the approach and recognizing that container queries are important to support. I'd argue isn't the whole point of the quick v4 to v5 to modernize Svelte? Then supporting modern CSS should be included in that and there is no reason to wait. Container queries is a very important development and the pathway to creating really complex and dynamic components.

There is also no burden in the planned switch to ESM / JSDoc. Swap the file extensions to .js and it will work. The css-tree extension code would be the same for Svelte 3.x, 4.x, 5.x until css-tree releases a version that supports modern query syntax. Keep in mind the mainline css-tree package is still being used; the CSS syntax supported is just extended in the interim. I in fact would be glad to thoroughly document the code in this PR in JSDoc if that helps alleviate visibility / understanding concerns about the code. Basically, what do I need to do to allay any concerns?

So I want to wait @continer supports in css-tree side as much as possible.

Do you have a date in mind? June this year? How long? Firefox shipped it last month; Firefox...

Already you released a forked version of Svelte. So people who need to support @container query, can use your forked version. If the volume of downloads is a lot, we can again consider merging this.

How many need to download it weekly? I should also note that the current mechanism for including my fork as svelte requires an inconvenient workaround in package.json. This is fine for end projects or those that want to test out the PR now, but doesn't work for component libraries much in the same way that your separate CSS file approach doesn't work.

I spent over a month last November developing a complex compound component (advanced color picker) that uses container queries. CQ is a game changer. I can't release that component in my library; it is commented out of the distribution. Folks can uncomment that under the node_modules package for my library and use the package.json override, but that is asking too much. I have a lot of other CQ plans as well.

It would be great to get other input and I do hope folks chime in within the window to get things into Svelte v3 then onto v4 / v5.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

We're currently leaning towards merging the PR because as you say it's the least worse option. Waiting: unclear how long we have to wait. Switching parsers: Probably even more work, and may have unforseen drawbacks.

I gave the PR a quick review and have a few questions

  • Can we be sure that these this methods you're using are not internal (I'm ok with semi-public) and can change at any moment? (CI should save us from accidentally pushin a broken version, but still)
  • Can you give maintainers write access to the branch? I wanted to do some quick formatting/snake_case code changes but can't
  • (also one more in a comment, see below)

@@ -0,0 +1,43 @@
// @ts-nocheck
// Note: Must import from the `css-tree` browser bundled distribution due to `createRequire` usage if importing from
// `css-tree` Node module directly. This allows the production build of Svelte to work correctly.
Copy link
Member

Choose a reason for hiding this comment

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

Could you expand on that createRequire usage thing? I'm not sure what this is about.

Copy link
Contributor Author

@typhonrt typhonrt Mar 17, 2023

Choose a reason for hiding this comment

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

Just repeating info I included in the post below:

So, the Node version of css-tree when including fork has a path that invokes createRequire in data.js:
https://github.com/csstree/csstree/blob/master/lib/data.js#L4-L7

The ESM bundled / browser version of css-tree has all of those resources in the ESM bundle thus doesn't depend on Node specific functionality to import a local JSON file.

When building Svelte for production (not local testing) it causes an issue to use the Node version of css-tree when accessing fork for extension as those JSON files can not be referenced by the production / zero-dependency build of Svelte as the bundling / distribution process doesn't include the internal css-tree JSON files referenced. When not producing a production build css-tree and a couple of other Node dependencies are not bundled / marked external thus it works.

https://github.com/sveltejs/svelte/blob/master/rollup.config.js#L144-L146

Copy link
Member

Choose a reason for hiding this comment

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

For Rollup this sounds ok. Just to double check: Once we go unbundled ESM then can't do this anymore like this. css-tree will be a regular dependency which means we have to use the public imports. But that would be ok because the JSON file will be available then within css-tree (because it's installed as a regular dependency), correct?

Copy link
Contributor Author

@typhonrt typhonrt Mar 17, 2023

Choose a reason for hiding this comment

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

Presumably when going unbundled ESM the dependencies previously bundled that are marked external in the local test build will be moved to package.json dependencies; likely: acorn, css-tree, and magic-string. In this case it will certainly be possible to directly reference the Node import path IE import { fork } from 'css-tree';

Outside of course SvelteKit taking the unbundled ESM approach, some day I'd be glad to learn more about the impetus for native ESM vs TS. I too have always been a native ESM proponent and all my Svelte UI framework / library work is native ESM. I also work on tooling to be able to generate bundled TS declarations from ESM source + JSDoc. Not saying that is applicable in Svelte's future, but a curious conversation overall for another day.

@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 17, 2023

Switching parsers: Probably even more work, and may have unforseen drawbacks.

There is no clear front runner for a better OSS parser. I have done an informal survey and most are still catching up to the latest standards. It would be a lot more work to replace the parser for sure.

  • Can we be sure that these this methods you're using are not internal (I'm ok with semi-public) and can change at any moment? (CI should save us from accidentally pushin a broken version, but still)

The this context that the individual AST node parsing modules receive when the parse function is invoked is the parser context which is defined in css-tree here:
https://github.com/csstree/csstree/blob/master/lib/parser/create.js#L89-L290

You can see the call invocation with the parser object as this context here:
https://github.com/csstree/csstree/blob/master/lib/parser/create.js#L333

This is the internal API exposed to the different AST node modules.

  • Can you give maintainers write access to the branch? I wanted to do some quick formatting/snake_case code changes but can't

I'd be glad to make the changes later today. I did use the code style found in css-tree; this PR and the extension process originated from my initial fork of css-tree / attempt to see if a PR could be merged w/ css-tree. I'm familiar w/ snake case and the use inside of Svelte for non-public API.

I also can look into giving access to maintainers, but this little cleanup task is something I can handle too.

Could you expand on that createRequire usage thing? I'm not sure what this is about.

So, the Node version of css-tree when including fork has a path that invokes createRequire in data.js:
https://github.com/csstree/csstree/blob/master/lib/data.js#L4-L7

The ESM bundled / browser version of css-tree has all of those resources in the ESM bundle thus doesn't depend on Node specific functionality to import a local JSON file.

When building Svelte for production (not local testing) it causes an issue to use the Node version of css-tree when accessing fork for extension as those JSON files can not be referenced by the production / zero-dependency build of Svelte as the bundling / distribution process doesn't include the internal css-tree JSON files referenced. When not producing a production build css-tree and a couple of other Node dependencies are not bundled / marked external thus it works.

https://github.com/sveltejs/svelte/blob/master/rollup.config.js#L144-L146

@dummdidumm
Copy link
Member

dummdidumm commented Mar 17, 2023

Re formating/snake_case - sure, you can also do it yourself 👍 If you're using VSCode, there's "convert indentation to tabs" which should help you - right now it's spaces mixed with tabs, and after conversion indentation is wrong in some places. Regarding snake_case, it's only a handful of variables/functions that would need a rename.

I'm so glad we're going to use Prettier for v4 which will make formatting stuff like this obsolete

@typhonrt
Copy link
Contributor Author

Re formating/snake_case - sure, you can also do it yourself 👍 If you're using VSCode, there's "convert indentation to tabs" which should help you - right now it's spaces mixed with tabs, and after conversion indentation is wrong in some places. Regarding snake_case, it's only a handful of variables/functions that would need a rename.

I'm pretty sure I got all the code formatting correct in last commit. Let me know if there is anything else I can improve to move things forward. I'm certainly OK w/ adding a healthy bit of JSDoc if that makes sense right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS container queries "@container" breaks parser
5 participants