Optimize Babel output a little bit #1656

Andarist · 2019-11-27T19:21:23Z

No description provided.

changeset-bot · 2019-11-27T19:21:25Z

🦋 Changeset is good to go

Latest commit: 87a07e5

We got this.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codesandbox-ci · 2019-11-27T19:22:46Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 87a07e5:

Sandbox	Source
Emotion	Configuration

packages/babel-plugin-emotion/src/utils/transform-expression-with-styles.js

codecov · 2019-11-28T07:35:01Z

Codecov Report

Merging #1656 into next will decrease coverage by 0.08%.
The diff coverage is 92.85%.

Impacted Files	Coverage Δ
packages/babel-plugin-emotion/src/utils/label.js	`98.11% <100%> (ø)`	⬆️
...bel-plugin-emotion/src/utils/get-styled-options.js	`100% <100%> (ø)`	⬆️
...tion/src/utils/transform-expression-with-styles.js	`100% <100%> (ø)`	⬆️
packages/utils/src/index.js	`100% <100%> (ø)`	⬆️
packages/babel-plugin-emotion/src/index.js	`92.04% <50%> (-2.01%)`	⬇️
packages/babel-plugin-emotion/src/utils/strings.js	`96.66% <94.11%> (+1.42%)`	⬆️

emmatown

I'd like to keep labels even in production because it's useful for some people to see them in production and if people really want to disable them in production, they can do autoLabel: false

Andarist · 2019-12-07T15:56:14Z

I'd like to keep labels even in production because it's useful for some people to see them in production and if people really want to disable them in production, they can do autoLabel: false

The same thing can be said about the reverse - if you want to see them in production, you can do autoLabel: true (not sure actually if it's true right now, but we could make it so).

My main motivation here is to optimize the default output - I always thought emotion is a huge proponent of such an approach with various hacks in the codebase just to make the output smaller.

I could live with this not being the default (as long as I can configure things how I like it - which is no labels in production). A problem in this particular case though is that we output opaque css objects with env check and what I put in this PR is my desired output. You assume that the party which transpiles files is the very same one that consumes it - but there are other use cases as well, such as building a library in which I'd like this dual output with labels being visible only during development.

So what I'm saying - maybe we need 3 values for autoLabel? true (always), false (never), 'auto'/undefined (with env checks). An additional question is - which one should be the default?

Andarist · 2019-12-14T12:58:19Z

@mitchellhamilton any more thoughts on this one? How I should proceed with this?

emmatown · 2019-12-15T22:22:39Z

always, never and auto sounds good to me

Andarist · 2019-12-16T06:31:32Z

SoautoLabel would become such enum rather than a simple boolean, right? What should be the default? I would vote on "auto"

emmatown · 2019-12-16T06:41:56Z

Agreed

Andarist · 2019-12-19T08:06:36Z

@mitchellhamilton could you review the code & existing tests? Once approved I'm going to adjust docs, add few simple explicit tests for possible autoLabel values and add a changeset

packages/babel-plugin-emotion/__tests__/__snapshots__/css-requires-options.js.snap

emmatown

Had an idea: Rename auto to dev because auto could really mean anything

Also, needs a changeset

Other than that, LGTM

emmatown · 2019-12-24T01:56:09Z

packages/babel-plugin-emotion/src/utils/arguments.js

+// @flow
+import { isTaggedTemplateExpressionTranspiledByTypeScript } from './checks'
+
+export const appendExpressionToArguments = (t: *, path: *, expression: *) => {


I think this replaces appendStringToArguments from src/utils/strings.js so we could we delete appendStringToArguments?

appendStringToArguments is still used here:

emotion/packages/babel-plugin-emotion/src/utils/transform-expression-with-styles.js

Line 149 in 3459ad7

appendStringToArguments(path, labelString, t)

both are very similar though so probably this could be unified somehow

Unifying them would probably be good. The main reason I brought this up though is that I saw that appendStringToArguments wasn't being run in the tests(https://codecov.io/gh/emotion-js/emotion/pull/1656/src/packages/babel-plugin-emotion/src/utils/strings.js) which probably isn't good

I've unified them, please take a look.

And the reason that function was not executed is that there were no tests for new autoLabel values (always and never), I've added them as well.

packages/babel-plugin-emotion/src/utils/arguments.js

Andarist · 2019-12-24T12:20:12Z

Had an idea: Rename auto to dev because auto could really mean anything

Maybe "dev-only"? It would be a little bit more descriptive

emmatown · 2019-12-31T07:26:50Z

Maybe "dev-only"? It would be a little bit more descriptive

SGTM

…ents

…ionToArguments

Andarist · 2020-01-01T10:50:35Z

I believe this is now ready to be merged, there are new tests, changesets, and updated docs.

* Optimize Babel output a little bit * Fix label regression in dev nodes * Add `autoLabel` validation * Update more snapshots * Fix flow error * Rename auto to dev-only as autoLabel value * appendExpressionToArguments -> appendStringReturningExpressionToArguments * Add simple tests for new autoLabel values * Merge appendStringToArguments logic into appendStringReturningExpressionToArguments * Adjust docs about new autoLabel values * Add changeset

Andarist commented Nov 27, 2019

View reviewed changes

packages/babel-plugin-emotion/src/utils/transform-expression-with-styles.js Show resolved Hide resolved

Andarist commented Nov 27, 2019

View reviewed changes

packages/babel-plugin-emotion/src/utils/transform-expression-with-styles.js Show resolved Hide resolved

Andarist commented Nov 27, 2019

View reviewed changes

packages/babel-plugin-emotion/src/utils/transform-expression-with-styles.js Outdated Show resolved Hide resolved

Andarist force-pushed the optimize-babel-output branch from 7d980ef to c8b7b04 Compare November 27, 2019 19:27

Andarist requested a review from emmatown November 27, 2019 19:27

Andarist force-pushed the optimize-babel-output branch from c8b7b04 to 5e0346d Compare November 28, 2019 07:26

emmatown reviewed Dec 2, 2019

View reviewed changes

Andarist force-pushed the optimize-babel-output branch 3 times, most recently from 4f29478 to caf9d21 Compare December 18, 2019 22:38

Andarist requested a review from emmatown December 21, 2019 10:30

emmatown reviewed Dec 23, 2019

View reviewed changes

packages/babel-plugin-emotion/__tests__/__snapshots__/css-requires-options.js.snap Outdated Show resolved Hide resolved

emmatown approved these changes Dec 24, 2019

View reviewed changes

emmatown reviewed Dec 24, 2019

View reviewed changes

packages/babel-plugin-emotion/src/utils/arguments.js Outdated Show resolved Hide resolved

Andarist added 4 commits January 1, 2020 11:49

Optimize Babel output a little bit

547c6a1

Fix label regression in dev nodes

d23ce55

Add autoLabel validation

063aedc

Update more snapshots

bb81925

Andarist added 7 commits January 1, 2020 11:49

Fix flow error

ca2e8c6

Rename auto to dev-only as autoLabel value

d700725

appendExpressionToArguments -> appendStringReturningExpressionToArgum…

3b72c55

…ents

Add simple tests for new autoLabel values

adab54c

Merge appendStringToArguments logic into appendStringReturningExpress…

6f9aac2

…ionToArguments

Adjust docs about new autoLabel values

00b06dd

Add changeset

87a07e5

Andarist force-pushed the optimize-babel-output branch from 989b72c to 87a07e5 Compare January 1, 2020 10:49

emmatown approved these changes Jan 5, 2020

View reviewed changes

emmatown merged commit c7850e6 into next Jan 5, 2020

emmatown deleted the optimize-babel-output branch January 5, 2020 00:22

github-actions bot mentioned this pull request Feb 29, 2020

Version Packages (next) #1735

Merged

github-actions bot mentioned this pull request Nov 10, 2020

Version Packages #2046

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize Babel output a little bit #1656

Optimize Babel output a little bit #1656

Andarist commented Nov 27, 2019

changeset-bot bot commented Nov 27, 2019 •

edited

codesandbox-ci bot commented Nov 27, 2019 •

edited

codecov bot commented Nov 28, 2019 •

edited

emmatown left a comment

Andarist commented Dec 7, 2019

Andarist commented Dec 14, 2019

emmatown commented Dec 15, 2019

Andarist commented Dec 16, 2019

emmatown commented Dec 16, 2019

Andarist commented Dec 19, 2019

emmatown left a comment

emmatown Dec 24, 2019

Andarist Dec 24, 2019

emmatown Dec 31, 2019

Andarist Jan 1, 2020 •

edited

Andarist commented Dec 24, 2019

emmatown commented Dec 31, 2019

Andarist commented Jan 1, 2020

Optimize Babel output a little bit #1656

Optimize Babel output a little bit #1656

Conversation

Andarist commented Nov 27, 2019

changeset-bot bot commented Nov 27, 2019 • edited

🦋 Changeset is good to go

codesandbox-ci bot commented Nov 27, 2019 • edited

codecov bot commented Nov 28, 2019 • edited

Codecov Report

emmatown left a comment

Choose a reason for hiding this comment

Andarist commented Dec 7, 2019

Andarist commented Dec 14, 2019

emmatown commented Dec 15, 2019

Andarist commented Dec 16, 2019

emmatown commented Dec 16, 2019

Andarist commented Dec 19, 2019

emmatown left a comment

Choose a reason for hiding this comment

emmatown Dec 24, 2019

Choose a reason for hiding this comment

Andarist Dec 24, 2019

Choose a reason for hiding this comment

emmatown Dec 31, 2019

Choose a reason for hiding this comment

Andarist Jan 1, 2020 • edited

Choose a reason for hiding this comment

Andarist commented Dec 24, 2019

emmatown commented Dec 31, 2019

Andarist commented Jan 1, 2020

changeset-bot bot commented Nov 27, 2019 •

edited

codesandbox-ci bot commented Nov 27, 2019 •

edited

codecov bot commented Nov 28, 2019 •

edited

Andarist Jan 1, 2020 •

edited