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

Packages: Move build config to calypso-build package #30772

Closed
wants to merge 11 commits into from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Feb 13, 2019

First steps. See p9oQ9f-87-p2 for context. Somewhat similar to WordPress/gutenberg#13814 -- ideally we'll converge at some point.

Jetpack counterpart PR: Automattic/jetpack#11344

Changes proposed in this Pull Request

TODO:

  • Make work for JP
  • Separate package for style assets?
  • Use for Calypso itself
  • ...

Tasks we'll need to do to follow through with this

  • Our (S)CSS buildchain. Depends on stuff in assets/stylesheets/_shared, which I'm currently directly including. Wondering if that should be its own package that the calypso-builder package can depend on. Est: 3d
    • To make things more fun, this is run through node-sass during build (and our files subsequently depend on the resulting public/custom-properties.css). If we turn this into an npm, this would need to be part of the published package.
    • We could maybe drop the SASS compilation on this one and just turn it into CSS? It's mostly just custom properties definitions after all.
  • Need to parametrize root dir in webpack config (instead of hard-wired __dirname). Est: .5d
  • References to Calypso's config. E.g. used for env, project name,... Hopefully avoidable or parametrizable. Est: 1d
  • Factor out a lot of Calypso specific config that doesn't carry over, e.g. server (section) loaders. Est: 2d
  • 'Actual' code from Calypso (that we'd need to publish as npms -- regardless of which build tool we end up using in JP). Not a lot to see here:
    • components/jetpack-logo
    • lib/format-currency/currencies
    • lib/route/path
    • lib/simple-payments/constants
    • lib/simple-payments/utils
  • Est: .5d if we copy/duplicate, 3d if we package
  • Stuff relevant for client/extensions. Is any of that even still used? Can we yank it out altogether? (This is probably minor/optional)

Testing instructions

See Automattic/jetpack#11344

/cc @Automattic/team-calypso @Automattic/gutenpack

@ockham ockham self-assigned this Feb 13, 2019
@matticbot
Copy link
Contributor

@jsnajdr
Copy link
Member

jsnajdr commented Feb 21, 2019

I like the idea to publish the SCSS build scripts and configs as a standalone package. The common SCSS definitions (mixins, functions, variables) can live there. Also the PostCSS and webpack loader configs.

The custom-properties.css built file can be shipped as part of the published package. We now build it as part of the webpack workflow, but that could be moved to a NPM script? install, postinstall, build, prepublish, prepare -- whichever is the right one 😆

We could maybe drop the SASS compilation on this one and just turn it into CSS? It's mostly just custom properties definitions after all.

The sources use a lot of SASS variables (all the $muriel-* ones) and functions (hex-to-rgb). I don't see a straightforward way to convert them to CSS. Shipping the CSS as part of the published NPM package is a more viable option.

@simison
Copy link
Member

simison commented Feb 21, 2019

Depends on stuff in assets/stylesheets/_shared, which I'm currently directly including.

Might be good to have a closer look at what we're actually depending on in blocks and if changing some Calypso variables/mixins to core equivalents makes more sense?

@ockham
Copy link
Contributor Author

ockham commented Feb 21, 2019

Depends on stuff in assets/stylesheets/_shared, which I'm currently directly including.

Might be good to have a closer look at what we're actually depending on in blocks and if changing some Calypso variables/mixins to core equivalents makes more sense?

Yep -- but in terms of moving forward, I'd play the transitivity card again: IIUC, a lot of these are based on a8c-color-studio, so it's unlikely that we can easily phase out our style basics altogether soon. We can however start depending on Gutenberg stuff and remove redundant definitions from ours.

@ockham
Copy link
Contributor Author

ockham commented Feb 21, 2019

Thanks @jsnajdr , makes sense and confirms what I had vaguely in mind!

The sources use a lot of SASS variables (all the $muriel-* ones) and functions (hex-to-rgb). I don't see a straightforward way to convert them to CSS. Shipping the CSS as part of the published NPM package is a more viable option.

Not immediately relevant, but maybe it'd be possible to propagate some changes upstream (to color-studio)? I.e. have them change their SASS vars to CSS custom properties? (Not sure about the functions.)

@jsnajdr
Copy link
Member

jsnajdr commented Feb 21, 2019

Not immediately relevant, but maybe it'd be possible to propagate some changes upstream (to color-studio)? I.e. have them change their SASS vars to CSS custom properties?

That could work: the color-studio package already exports the color data in several forms: JSON, SASS variable definitions, CSS classes. Adding a new color-css-variables.css file with definitions like

--color-muriel-purple-0: #f7f5f8;

should be fairly easy.

The question is whether the postcss-custom-properties plugin will work with that. Needs to be tested.

@ockham
Copy link
Contributor Author

ockham commented Feb 21, 2019

Seems like it has been brought up in that repo's issue tracker even: Automattic/color-studio#3

@blowery
Copy link
Contributor

blowery commented Feb 21, 2019

There are some other big bits that currently use SASS that are hard to replace with custom props:

  • The z-index map (this makes my head hurt, though ideally components wouldn't have to mess with z-index at all?)
  • media queries (ditto)

@ockham
Copy link
Contributor Author

ockham commented Mar 12, 2019

Closing. This has served its purpose to inform pafL3P-kB-p2 and https://github.com/orgs/Automattic/projects/80.

@ockham ockham closed this Mar 12, 2019
@ockham ockham deleted the try/calypso-build-package branch March 12, 2019 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Framework [Goal] Gutenberg Working towards full integration with Gutenberg Jetpack Packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants