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

docs(meetings): add additional notes stuff #2

Merged
merged 5 commits into from
Jun 1, 2017

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented May 18, 2017

Here it is folks!

thx for answering etc in Advance

@Hotell Hotell force-pushed the may-meeting-notes-additions branch 2 times, most recently from 3a977be to ce12bee Compare May 18, 2017 23:41
@Hotell Hotell changed the title docs(meeings): add additional notes stuff docs(meetings): add additional notes stuff May 18, 2017

## @hotell questions

- since `webcomponents.js` **1.0** is out ( YES ALSO on NPM finally !!!) we can drop our custom skatejs/webcomponents ? ( our custom ones doesnt work with Angular )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great idea. Have they made lazy-loading the necessary polyfills simple? The main issues I've had with it is the sheer amount of friction to just get a cross-browser simple example working in a reasonable amount of time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try it out and will let you know ( I need this for demo of my upcoming talk )

Copy link
Contributor Author

@Hotell Hotell May 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works like a charm, although slightly complicated config

this is dev mode === not minified/gzipped
-> also webpack is putting it's HMR etc in that polyfill.js file ( probably that's why it's so big )

overall zero kB on browsers with full support, what is unfortunate is that it loads HTML Import shim which is very huge - maybe worth to fork and tweak that loader for own particular needs -> for skate projects ? WDYT

image


- since `webcomponents.js` **1.0** is out ( YES ALSO on NPM finally !!!) we can drop our custom skatejs/webcomponents ? ( our custom ones doesnt work with Angular )

- what will/is the new size skate 5.x + preact 8.x as renderer ( marketing )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skate is something like 3.6k or something. I think general convention here is to round down, just to make things look better, but I'm not fussed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to give a "size with Preact" number as then we have to maintain that along with Jason reducing his bundle size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup agreed ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image


- what will/is the new size skate 5.x + preact 8.x as renderer ( marketing )

- what type of source are we gonna ship ?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, just UMD (in umd/) + ES2015 (in es/). The es/ will be kept up to date with the latest ES spec, within reason. We still need to figure out whether we bump major versions as specs become finalised.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I tweeted recently, I think this approach is not completely ok. We definitely need the module in package json for es6 modules + es5 transpiled and es2015 for just esNext. But we can iterate in feature release or so, no pressure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, okay. I think that's simple enough to provide because all we need to do is:

  1. Re-enable the esModule build with NWB
  2. Change the es/ output directory for the Babel step to es-next/ or es-latest/.
  3. Bikeshed the name of the new directory because it becomes API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deal!

- UMD
- ES 2015 + modules ( three shakable or can be used as is without native-shim)

# PR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

This is awesome! I like that we add stuff like this here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha PR is important in OSS, stupid projects like cat feeder etc have 5k stars ehm ehm, I really thing that this project is revolutionary and really the only sane way how to write WC for anyone ( who prefers reactive/functional way) ( especially with turned off ShadowDOM for enterprise/legacy apps - which need to support IE9 and so on )

@@ -19,11 +22,330 @@ Attendees
- Would be good to have Flow in sooner rather than later.
- If TypeScript definitions change if we merged it later, then it could possibly break (low risk).

## 5.x recap
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thank you so much for compiling this information!


Attendees

- @matthewp
- @treshugart
- @hotell ( offline after meeting notes )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: inconsistent parenthesis spacing (mostly trolling :p)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:D well I written this down at 2 am after watching alien:covenant so :D 🗡


### Component reference via Constructor

> isn't possible in 5.x ( Preact doesn't recognises custom-element via CTOR )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@developit (I'm being lazy) are there any plans to implement this? I'd be happy to contribute it if there's nothing in the pipeline yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created issue for that preactjs/preact#704

define(MyComponent)
```

**5.x**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon we should simplify these examples as much as possible to highlight the exact change. Like even just a <Hello /> example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I was thinking too, to just provide diff but having whole example is easier to grook. What we can do is to have the diff at first and then both version whole examples WDYT ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe simple for showing the diff, and at the end show an example with all the changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk I'm on it


### return Array as root node

> isn't possible in 5.x ( Preact doesn't support rendering arrays as React Fiber for instance )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I could put money on it, it's in the works. We could patch the Preact renderer (src/with-component.js) in Skate to allow this, if we really wanted.

Copy link
Contributor Author

@Hotell Hotell May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I saw some retweet by @developit that someone has a working prototype but perf is terrible at the moment ( according to that tweet )

Hard to say if we should monkey patch it hmm, I guess we should for those components for layout, also easier migration from v4

### return Array as root node

> isn't possible in 5.x ( Preact doesn't support rendering arrays as React Fiber for instance )
> Critical refactoring especially for apps that are using custom-elements for layout via Flexbox
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true. Maybe we should put it in Preact?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is in Preact docs already no ?

... Nope, there isn't any mention in the docs https://preactjs.com/guide/lifecycle-methods

### return Array as root node

> isn't possible in 5.x ( Preact doesn't support rendering arrays as React Fiber for instance )
> Critical refactoring especially for apps that are using custom-elements for layout via Flexbox
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it'd be best to have a compat package that can make things smoother. As mentioned in previous meetings, you can have several versions of Skate side-by-side, so I'm not sure this is a huge issue. Could also write code shifts for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot write code shifts for css AFAIK

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right because returning arrays is only half the battle. Don't think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I guess we have to be consistent and don't provide surprises for user who would use they custom renderer which may not support returning arrays as root ? dont know hmm

define(MyComponent)
```

**5.x**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't spot the breaking change here. Can we simplify this to the most simple form (should probably do this for all examples)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

props( this, {myArray: ['hello']} ) vs this.props = { myArray: ['hello'] }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! 🍻

## Flow

- Flow can be merged sooner rather than later if we can get `get-typed` published.

- need explanation of TS generated files by Flow secret tool before merging as LGTM types, see -> https://github.com/skatejs/skatejs/pull/1163#discussion_r116605816
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean here is that we can merge and still maintain the TS types until that is done, but I agree, I think it might be better to hold off on that and make the integration seamless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cheers on that !

@Hotell Hotell force-pushed the may-meeting-notes-additions branch from 9e159e9 to 3b26210 Compare May 22, 2017 20:57
@Hotell
Copy link
Contributor Author

Hotell commented May 22, 2017

allright added diffs, custom base creation, links ...

pls re-review , if LGTM we can merge and iterate further if needed

@Hotell
Copy link
Contributor Author

Hotell commented May 23, 2017

huh so I've migrated my starter-kit and that issue with Flex layouts and not be able to render array as root is really "painful" :( Hotell/skate-starter#1

@treshugart
Copy link
Member

Can't you instead of applying flex stuff to :host, just do it to the wrapping div?

@Hotell
Copy link
Contributor Author

Hotell commented Jun 1, 2017

can we merge this and iterate on more merging diffs please ?

I will start upgrading docs as well

@treshugart treshugart merged commit 24cd8ea into master Jun 1, 2017
@treshugart treshugart deleted the may-meeting-notes-additions branch June 1, 2017 16:42
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