cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.07k stars 397 forks source link

Typescript Refactoring #1053

Open jeremy-coleman opened 5 years ago

jeremy-coleman commented 5 years ago

https://github.com/jeremy-coleman/jss

will someone with a knowledge of the codebase please fix what you can in the src/jss folder.

kof commented 5 years ago

I appreciate the effort, but we haven't decided yet on this in #1035

jeremy-coleman commented 5 years ago

regardless, take a look at it please - its pretty much done

TrySound commented 5 years ago

What is done? I see you only commited build artifacts.

jeremy-coleman commented 5 years ago

look closer - src/

jeremy-coleman commented 5 years ago

imo the big things that need a look are 1) the several different and conflicting rule objects 2) consider renaming StyleSheet to JssStyleSheet. 3) consider removing Virtual Renderer

btw this is typescript/flow agnostic

for #1, the following seems mostly sufficient - and delete all the other ones

export type JssOptions = {
  createGenerateId?: (rule: Rule, sheet?: StyleSheet) => string,
  plugins?: Array<Plugin>,
  insertionPoint?: InsertionPoint,
  virtual?: Boolean,
  Renderer: Renderer // or..
  Renderer2: DomRenderer | VirtualRenderer 
}

export type RuleOptions = {
  selector?: string,
  scoped?: boolean,
  sheet?: StyleSheet & any,
  index?: number,
  parent?: Rule & any,
  classes?: Classes,
  keyframes?: KeyframesMap,
  jss?: Jss,
  generateId?: GenerateId,
  Renderer?: Renderer,

  //stylesheet
  media?: string,
  meta?: string,
  link?: boolean,
  element?: HTMLStyleElement,
  classNamePrefix?: string
  //parent: ConditionalRule | KeyframesRule | StyleSheet,
}

and for the Rule, this is one possibility - ofc ideal would be to have at and type as a tuple, but less restriction is better sometimes

type ConditionalAtRule = "@media" | "@supports"
type SimpleAtRule = '@charset'|'@import'|'@namespace'
type ViewportAtRule = '@-ms-viewport' | '@viewport'

export type Rule = {
  //base
  type: unknown | 'viewport' | 'style' | 'fontface' | 'conditional' | 'font-face' | 'keyframes' | 'simple';
  key: string;
  isProcessed: boolean;
  style: JssStyle;
  options: RuleOptions;

  at?: unknown | '@font-face' | '@viewport' | '@keyframes' | ConditionalAtRule | SimpleAtRule | ViewportAtRule;

  renderable?:  Object | null | void | undefined 
    |CSSStyleRule  //StyleRule
    |CSSKeyframesRule  //Keyframe
    |CSSMediaRule //conditional
  ;

  toString(options?: ToCssOptions): string;
  selectorText?: string;
  id?: string | undefined | null;
  name?: string;
  prop?(name: string, value?: JssValue, options?: UpdateOptions): Partial<Rule> | string
   rules?: CSSRuleList
  //rules: RuleList;
}
Jack-Works commented 5 years ago

https://github.com/cssinjs/jss/blob/master/packages/jss/src/index.d.ts#L36

Renderer: Renderer // or.. is wrong. This doesn't mean a class that implements Renderer but an instance of a class that implements Renderer. Consider this (or other better way)

Renderer: { new(): Renderer }
kof commented 4 years ago

@HenriBeck is this issue/linked repo useful? If not feel free to close.

jeremy-coleman commented 4 years ago

The repo isnt useful, since it doesnt have the recent updates , i can re rewrite the latest jss to ts if wanted though. If not pls do close