paypal/glamorous

Hi there 👋, welcome! Please introduce yourself, and if you have a question, please use this: https://help.glamorous.rocks

Andrew Blick

@blicksky

This is the line where glamorous fails to detect that the prop-types module is available, because it's attempting to use require() as opposed to accessing the PropTypes global: https://github.com/paypal/glamorous/blob/master/src/react-compat.js#L80

Despite it being listed as an external global for rollup to replace: https://github.com/paypal/glamorous/blob/master/rollup.config.js

Andrew Blick

@blicksky

If I'm right about what's happening, then when React 16 drops React.PropTypes, this will stop working altogether

Andrew Blick

@blicksky

It looks like the documentation for how to use the stand-alone distributable that was in the README was removed in https://github.com/paypal/glamorous/commit/90f8334be3e5d49906861da789f2176d5421a6e4#diff-04c6e90faac2675aa89e2176d2eec7d8

Kent C. Dodds

@kentcdodds

Have you tried the UMD build? See all the distributables here: https://unpkg.com/glamorous@4.9.3/dist/

Also, the preact-compaitible builds: https://unpkg.com/glamorous@4.9.3/preact/dist/

Andrew Blick

@blicksky

Yeah, in my setup where I found this, and the codesandbox example I link to, it's using the UMD build

and I'm not trying to use this with Preact

here's the warning with the stacktrace:

VM63 react.js:2277 Warning: Accessing PropTypes via the main React package is deprecated, and will be removed in  React v16.0. Use the latest available v15.* prop-types package from npm instead. For info on usage, compatibility, migration and more, see https://fb.me/prop-types-docs
printWarning @ VM63 react.js:2277
lowPriorityWarning @ VM63 react.js:2296
get @ VM63 react.js:275
(anonymous) @ VM67 glamorous.umd.js:341
(anonymous) @ VM67 glamorous.umd.js:4
(anonymous) @ VM67 glamorous.umd.js:5

and glamorous.umd.js:341 points me to:

Andrew Blick

@blicksky

} else if (parseFloat(React.version.slice(0, 4)) >= 15.5) {
  /* istanbul ignore next */
  try {
    _PropTypes = require('prop-types');
    /* istanbul ignore next */
  } catch (error) {
    // ignore
  }
}
/* istanbul ignore next */
_PropTypes = _PropTypes || React.PropTypes;

debugging shows me that React.version is greater than 15.5 (also loaded via React's stand-alone distributable), it tries and fails to call require('prop-types'), and then it falls back to React.PropTypes, triggering the warning

Andrew Blick

@blicksky

as an experiment, I manually edited the file to change that last line to _PropTypes = _PropTypes || PropTypes || React.PropTypes; and that works, but that's clearly not the right solution

I think I'll raise a GitHub issue with these details

Andrew Blick

@blicksky

I created paypal/glamorous#334

Jeppe Reinhold

@JReinhold

Is there a way to dynamically change the theme in a ThemeProvider, eg. having a toggle button that changes the theme from light to dark, and then all components that rely on that theme will update? Right now, I can change the theme prop just fine for my top level ThemeProvider, but it doesn't trigger the components to re-render (probably because it is passed as context), so I don't actually see the new changes.

Luke John

@luke-john

hey @JReinhold, you should be able to do that. we take advantage of this in our internal component library currently. I've setup a simple example at https://stackblitz.com/edit/glamorous-theme-switch?file=index.tsx

(and without typescript at https://codesandbox.io/s/qz1x626mzq 😉)

Jeppe Reinhold

@JReinhold

Oh wow, thanks! Will take a look immediately. Maybe it's because my top-level component is an SFC and not a class. and thanks for the TypeScript example, exactly what I wanted ;)

Gustav Wengel

@GeeWee

Hi. I'm the one using what @JReinhold was asking about.

Can you explain how that works? I've managed to get it working just fine, but I don't understand why. If I put my glamorous component inside e.g. a PureComponent. The pureComponent doesn't rerender when the theme is updated, so the children shouldn't either, should they? But they do rerender when changing the theme. I'm reasonably sure changing the context doesn't force any rerenders. Does glamorous keep an internal list of components that needs to update when the theming context switches, or is there something fundamental I'm misunderstanding?

Luke John

@luke-john

hey @GeeWee, glamorous is providing the theme via context, however it is providing a subscription service (via brcast) through context https://github.com/paypal/glamorous/blob/master/src/theme-provider.js

and all glamorous components are subscribed to this via the with-theme helper https://github.com/paypal/glamorous/blob/master/src/theme-provider.js.

Gustav Wengel

@GeeWee

Ah, that makes sense. I was beginning to question my entire mental model of how React worked. Thanks a lot for the help!

Luke John

@luke-john

Thanks for asking the question, I'd never actually looked into how that worked with glamorous :)

Gustav Wengel

@GeeWee

Guess we both got smarter :D

Kent C. Dodds

@kentcdodds

:heart:

Philipp Kursawe

@pke

Still undecided between styled-components and glamorous. I don't like that I have to create "dummy" wrapper styled-components

but what about runtime performance, browser compat of the generated stylesheets and bundle size? Are there any numbers our there?

Kent C. Dodds

@kentcdodds

All the popular solutions are fast enough for 99% of use cases, but if you really care about size and speed, then use emotion

Andrew Smith

@andrew-codes

Afternoon, I am trying to contribute to the glamorous-website repo, but am running into a bit of a snag. A fresh pull down from the non-fork repo and running the command npm start validate fails. Test are failing (6) and I wanted to see if I am missing something before opening an issue. I'm certain there is something I missed, but nothing is standing out as of now. Thanks.

Andrew Smith

@andrew-codes

If it is a known issue, I'll create an issue and see if I can resolve it; however, because it is a fresh pull down of master I am figuring I'm missing a step of some kind.

Luke John

@luke-john

Hey @andrew-codes thanks for reporting this issue (and trying to contribute 🙂), I've just rebased off the latest master and don't seem to have the same issue 🤔. I'm unlikely to get time to help investigate this today, if you're able to create a new issue on the repository that would be appreciated thanks. I suspect more people will be looking at those issues than watching this gitter channel 🙂.

Kent C. Dodds

@kentcdodds

@andrew-codes, it looks to me like that's a syntax error. The problem is probably that you're on an old version of node. You must use version 8 of node to run this repository.

Thanks for contributing!

Andrew Smith

@andrew-codes

Hmm, I was running 8.6.0 via nvm, but am trying with 8.7.0 right now and I'll let you know how it goes. Thank you @kentcdodds :-)

Thank you @luke-john

node@8.7.0 gives me the same errors when running the validate or test commands.

I will file an issue on GH. Running npm start test yields a more descriptive error message. I suspect there is something environment that is not setup on my machine in some way (a globally installed package, etc.). Whatever the cause is, it would be at least good to document the outcome for others; surely I'm not the only one with this problem.

Andrew Smith

@andrew-codes

sorry, it gives me different errors.

I think the previous one must have been node@6. With node@8 I see failing tests around algoia-config.js and macros. Here let me post a screenshot.

I cropped the image for brevity

Rich Dillon

@richraid21

Howdy fellow Reacters! I was reading through the docs, and the library looks great. I'm wondering if there is any discussion or best practices regarding when to use a prop to change a style vs when to just create a new element with that style

Ie, <Link prop='a choice'> vs AChoiceLink

Luke John

@luke-john

I've not seen any discussions of that nature specific to glamorous. The pros and cons of each would be fairly similar to other React components though. I've put down some very initial thoughts at https://gist.github.com/luke-john/b80923d5b4084c753a7e1120695d5d85 on some pros and cons specific to glamorous, but these are mostly initial notes to be fleshed out.

I'm going do some more thinking on it, discuss with my team tomorrow and will update with a more thought out and reasoned list 🙂. Also the reasonings/discussions are likely to be similar across css-in-js libraries so you may find good guidelines/discussions elsewhere as well 🙂.

Jeppe Reinhold

@JReinhold

I'm trying to define a grid-template-area using Glamorous, which relies on the values being multiline. I just want to make sure I'm doing it correctly.

this is the CSS way of doing grid-template-area:

  grid-template-areas: "head head"
                       "nav  main"
                       "nav  foot";

this is what I'm doing now in Glamorous that works:

gridTemplateAreas: `'head head' \n
                    'nav main' \n
                    'nav foot'`,

notice the template string with strings inside and newlines, a bit hacky. is there another way to write it?

Kent C. Dodds

@kentcdodds

@JReinhold, did you notice the CSS Grid example ? https://glamorous.rocks/examples#css-grid

Where communities thrive

People

Repo info

Activity