construct-style-sheets icon indicating copy to clipboard operation
construct-style-sheets copied to clipboard

feat: add support for the latest spec

Open Lodin opened this issue 2 years ago • 8 comments

Resolves #124.

This PR adds support for the latest version of the specification (Proxy(Array) instead of FrozenArray). I have also done a refactoring to put all the code in the regular classes; they are transformed by Babel during the rollup build. To reduce the common size, I used terser compilation, so the resulting file is minified. According to size-limit, the size of the package now is 2.67 Kb, and the overall approach looks better than before.

Lodin avatar Jun 05 '23 18:06 Lodin

Codecov Report

Merging #126 (5830a38) into main (e6c5177) will increase coverage by 1.93%. The diff coverage is 98.44%.

:exclamation: Current head 5830a38 differs from pull request most recent head 4bc5d81. Consider uploading reports for the commit 4bc5d81 to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #126      +/-   ##
==========================================
+ Coverage   92.06%   94.00%   +1.93%     
==========================================
  Files           5        6       +1     
  Lines         189      200      +11     
  Branches       32       29       -3     
==========================================
+ Hits          174      188      +14     
- Misses          4        8       +4     
+ Partials       11        4       -7     
Flag Coverage Δ
unittests 94.00% <98.44%> (+1.93%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/index.ts 86.66% <ø> (ø)
src/shared.ts 50.00% <ø> (-33.34%) :arrow_down:
src/ConstructedStyleSheet.ts 97.01% <96.55%> (+2.72%) :arrow_up:
src/AdoptedStyleSheetsArray.ts 100.00% <100.00%> (ø)
src/Location.ts 94.56% <100.00%> (+2.61%) :arrow_up:
src/utils.ts 90.90% <100.00%> (ø)

codecov-commenter avatar Jun 05 '23 19:06 codecov-commenter

LGTM

Except in the imports. you directing the imports to an js files, it would be way better if the imports actually link to the typescript and not some build files, as this could allow a build free use in deno.

Btw Would be cool seeing a new release when this lands

lucsoft avatar Jun 14 '23 15:06 lucsoft

@lucsoft, you mean you'd like to have a adoptedStyleSheets.ts file alongside the built JS file?

Lodin avatar Jun 16 '23 12:06 Lodin

@calebdwilliams any updates when this will land?

lucsoft avatar Oct 02 '23 12:10 lucsoft

Yikes I’ve been missing emails on this. I’ll review today and push.

calebdwilliams avatar Oct 02 '23 13:10 calebdwilliams

@Lodin tests seem to be failing on MacOS. Code looks mostly fine but I didn't do a deep dive due to the tests. Once that's complete, I'd be happy to merge.

calebdwilliams avatar Oct 02 '23 13:10 calebdwilliams

Thanks for the review. It's a bit hard for me to find time to finalize the PR but I'll try to do it ASAP.

Lodin avatar Oct 02 '23 14:10 Lodin