Skip to content

Conversation

@davidgovea
Copy link
Contributor

@davidgovea davidgovea commented Aug 27, 2020

Hi @TVke !

This PR is not ready yet, but I wanted to share the approach to discuss.

Motivations:

  1. TypeScript definitions for non-default tailwind configs (Issue Types for custom configs // Tailwind philosophy #18)
    • Pre-generation was the only avenue that I could think of here
  2. Eliminate project structure assumptions for requiring tailwind.config.js: We ship react-native-based native SDKs with electrode-native, and in that case, the require('../../../tailwind.config') causes issues (here: https://github.com/TVke/react-native-tailwindcss/blob/master/util/configHandler.js#L6).

Definitely acknowledge that 2) is more of an exotic build setup issue, but 1) is of high importance to all users of this library

Approach:

  1. Use react-native-tailwindcss as a generator, not a runtime dependency:
    • babel-node generate.js --config tailwind.config.js
    • a rn-tailwind.ts file is created
  2. import { t, color } from './rn-tailwind'

Since the generated file is TypeScript, we have full types, even for custom changes to tailwind config!

Design notes:

  • This PR uses some hacky approaches to avoid any edits to the existing library
    • rewiremock is used to mangle internal require calls
    • The output TS file is manually built with string interpolation
  • babel-node is used to work with ES modules in node -- I was lazy -- this part needs work

Further work:

  • Consider the dev UX for this static file
    • Since using tailwind already requires the addition of the config file, adding one more local file for proper typings is justifiable, IMO
    • Can it be aliased in the project, to avoid relative path import hell?
  • Consider generating a d.ts file only, and maintaining runtime generation via existing library
    • What is the impact of generating at runtime? CPU time, memory? It's possible that the static approach is superior.
  • Consider purging unused styles in production. This is easier in javascript-land than it usually is, but how would we make it not manual? Custom metro bundler config tweaks?

@davidgovea
Copy link
Contributor Author

This has been working out really well -- Actually caught some type errors, since this goes beyond the styleName: any definitions of the current version.

For example, passing <View style={[t.alignCenter]} /> now causes a type error: textAlignVertical: 'center' is not a valid style for View. I actually intended to use itemsCenter.

I ended up using module-aliasing to trample the "real" react-native-tailwindcss with my static file:

tsconfig.json

{
  "compilerOptions": {
    "paths": {
      "react-native-tailwindcss": ["rn-tailwind.ts"]

babel.config.js

module.exports = {
  plugins: [
    ['module-resolver', {
      root: ['.'],
      alias: {
        'react-native-tailwindcss': ['./rn-tailwind.ts'],
      },
    }],

Then, all my existing imports worked, with the enhanced typing (and my custom keys like t.bgBrand)
import { t } from 'react-native-tailwindcss';
import { t, color } from 'react-native-tailwindcss';
I never import { theme }, but should probably handle that too.

@davidgovea
Copy link
Contributor Author

Started investigating the performance characteristics of static-vs-runtime tailwind.

Created 2 fresh expo apps, one with the current runtime generated tailwind, and one with a single static-file version (using the default tailwind config). Evaluated both with react-native-bundle-visualizer.

Static-generated react-native-tailwindcss
Overall bundle size: 923.94 KB

  • rn-tailwind.ts: 41.44 KB

Runtime-generated:
Overall bundle size: 945.34 KB

  • reduce-css-calc: 29.39 KB
  • react-native-tailwindcss: 23.94 KB
  • postcss-value-parser: 3.38 KB
  • css-unit-converter: 1.24 KB
  • additional lodash modules: +4.81 KB

Those numbers don't add up perfectly -- it's not clear to me where the additional bundle size is coming from in the runtime-generated mode.


Learnings:

  • Runtime generation adds quite a few additional dependencies.
  • The static-generated tailwind file is pretty big

Thoughts:

  • Memory usage should be profiled. But I would guess that static-generation is more efficient, since the runtime approach still generates the full style object in memory.
  • The generated file is very "gzippable", since many strings are repeated. A quick test shows an ~80% reduction in size, from 50K to just under 10K when gzipped. So, the sizes added to the resulting app archive will not be high.

It seems to me that pre-generation is the best (only?) way to get TypeScript definitions for custom tailwind configs. So, if we're pre-generating types, why not also pre-generate the style object?

@TVke would be interested in your thoughts here. This would be a significant change to the approach taken by this library.

One way we could keep current behavior, but allow opt-in to pregeneration, is using a postinstall hook:

npx react-native-tailwindcss-generate could consume the project's tailwind.config.js file and trample the "main" node_modules/react-native-tailwindcss/index.js & index.d.ts files.

This could be added to a project's "postinstall" hook to ensure the pregenerated file is always present and up-to-date. No babel/tsconfig path hacks needed.


This is shaping up to suit my needs perfectly. If you agree, I will continue working on this in this repo. Otherwise, I could make a new project that depends on rn-tailwindcss and performs this generation separately. In any case, definitely want to leverage the great work you've done on adapting tailwind styles to RN.

Thanks for reading!

@TVke
Copy link
Owner

TVke commented Sep 28, 2020

Hi @davidgovea

I would love the autocomplete to work as it should so I think you are heading in a good direction.
I was a bit concerned about the need for building but things like that are easily implemented in the build commands 😊

greetings
Thomas

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants