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

Add unpkg key to package.json #936

Closed
wants to merge 3 commits into from

Conversation

appsforartists
Copy link
Member

@appsforartists appsforartists commented Dec 13, 2018

This provides a completely bundled distribution, ready for importing from CDNs like unpkg.com for use in sandboxes like Codepen. It allows this code to work natively in the browser:

import { create as createJSS } from 'https://unpkg.com/jss';

The PR is a work-in-progress, but I'd rather start a conversation with an example than with prose. As it is, I can't get sizeSnapshot to work with the bundled version.

I also wonder how best to handle plugins/presets. The easiest options would be to add unpkg entry points to each package, and force authors to use multiple imports:

import { create as createJSS } from 'https://unpkg.com/jss';
import createDefaultJSSPreset from 'https://unpkg.com/jss-preset-default';

I wonder what the best way to produce a batteries-included bundle is. Perhaps there should be another bundle jss-all that exports all the other packages. Then usage would be:

import * as jssAll from 'https://unpkg.com/jss-all';

const jss = jssAll.create(jssAll.preset());

Todo

  • Add unpkg field to every package
  • Understand and fix the sizeSnapshot crash
  • Make a jss-starter-kit package.
    • console.warn that it's for learning and prototyping only, not bundled for production use.
  • Changelog
  • Docs here

@appsforartists
Copy link
Member Author

I believe this would also enable me to post a mirror of JSS here:

https://developers.google.com/speed/libraries/

@appsforartists
Copy link
Member Author

Here's an example:

https://unpkg.com/material-motion loads material-motion.bundle.js because unpkg is set in package.json.

@HenriBeck
Copy link
Member

Please add to every package the unpkg key.

@kof
Copy link
Member

kof commented Dec 13, 2018

Wow didn't know about this. Def. makes sense to me, lets create a list what needs to be done here! I started adding todos in the description, feel free to do so too!

@TrySound
Copy link
Member

I think it make sense to bundle only commonjs packages. All left imports will be converted by unpkg to urls.

@TrySound
Copy link
Member

jss-plugin-isolate

  • css-initials

jss-plugin-syntax-camel-case

  • hyphenate-style-name

jss-plugin-syntax-compose

  • warning (not required if we will stop support ie9 and use tiny-warning)

jss-plugin-syntax-extend

  • warning

jss-plugin-syntax-nested

  • warning

jss-plugin-syntax-template

  • warning

jss-plugin-vendor-prefixer

  • css-vendor

jss

  • warning

react-jss

  • hoist-non-react-statics
  • prop-types

@kof
Copy link
Member

kof commented Dec 13, 2018

@appsforartists your material-motion.bundle.js seems to be a cjs with all the dependencies inside, right? I see no exports are used, so its basically bundled for a regular script loader, but we want to make it loadable with import, doesn't import require exports?

@kof
Copy link
Member

kof commented Dec 13, 2018

Ah found export in the end of the bundle. Now I wonder if it uses ES export, its not a cjs bundle, right?

@kof
Copy link
Member

kof commented Dec 13, 2018

Ok so its an esm bundle, I was just confused that you had no import statements at all.

@kof
Copy link
Member

kof commented Dec 13, 2018

Now I get what @TrySound is saying. Inlining all cjs, because we want esm bundle and can't make require calls there. ESM capable dependencies can be imported even over unpkg

@TrySound
Copy link
Member

Hm.. css-vendor is yours. This is cool :)

@TrySound
Copy link
Member

Ideally all small packages could support esm so we could specify unpkg as an alias to module field. So we will need to make esm bundle only for react-jss

@kof
Copy link
Member

kof commented Dec 13, 2018

@TrySound lets do it!

@appsforartists
Copy link
Member Author

I don't see the automatic URL rewriting @TrySound is talking about. See wobble here:

https://unpkg.com/material-motion@0.1.0/dist/interactions/NumericSpring.js

It's also better to bundle, even with ESM, to avoid network latency.

Anyone know anything about that sizeSnapshot crash?

@TrySound
Copy link
Member

@appsforartists
Copy link
Member Author

Magic!

I still think we should distribute full bundles. It reduces network latency and supports CDNs that don't do fancy rewriting (e.g. Google Hosted Libraries). Plus, it's really easy to implement.

@kof are you 👍 on making a jss-all to make everything importable from a single file?

@TrySound
Copy link
Member

TrySound commented Dec 13, 2018

This is just a feature for playing in sandboxes. Network latency should not matter here. There are huge and small modules. Some tool like bundler should still optimize them. Raw modules will be always a problem for production.

@kof
Copy link
Member

kof commented Dec 13, 2018

@appsforartists What would be the use cases for bundling it all? I can imagine providing a react-jss bunde, jss core bundle, jss presets bundle and the rest separartely if at all. There are some plugins which are not part of the default preset (e.g. jss-isolate)

@appsforartists
Copy link
Member Author

I mainly care about being able to import jss and preset from a single bundle, and was looking for a way to accomplish that. Since the main use case is for Codepens et. al., I haven't been that concerned about bundle size.

Looks like there's a multientry plugin. We might be able to do

  inputs: ['jss/index.js', 'jss-preset-default/index.js'],
  output: 'jss-preset-default/dist/jss-preset-default.bundle.js'
  plugins: [..., multientry(), ...],

(generated/introspected for the packages we care about). It's a little weird to import jss from jss-preset-default, but this keeps the dependencies pointed the right way.

The reason I was thinking about something like jss-all was to have a single bundle that could import the others to keep cycles out of the dependency graph. If all is not a useful unit, we could do the same thing in smaller pieces: jss-starter-kit could be jss, jss-preset-default. react-jss-starter-kit could be that with react-jss.

Would you be 👍 to jss-starter-kit and react-jss-starter-kit as new packages that bundle the commonly used exports?

@TrySound
Copy link
Member

Why it's so hard to import 2 or 3 dependencies? Why another weird package for this? Why you care about network for playgrounds? All this stuff just adds more efforts for maintainers with no real reason.

Looks similar to wrapping react component just to hide a couple of boolean fields. Really weird and wasteful.

@kof
Copy link
Member

kof commented Dec 13, 2018

Oh starter-kit or something similar sounds ok, in that case we can put there anything. Mb something even more clear to make sure people don't use it and then blame for bundle size, like a jss-sandbox or something

@kof
Copy link
Member

kof commented Dec 13, 2018

@TrySound its an optimization for experimental and demo use, people don't want to care in that case what packages to use and how to setup, ideally for this use case there should be no setup reequired at all. Its the same reason why codesandbox is popular, it just works with one click.

@kof
Copy link
Member

kof commented Dec 13, 2018

I assume this won't cost us much maintenance time, otherwise I wouldn't want to do that either.

@appsforartists
Copy link
Member Author

Oleg stated my POV pretty well. Having an easy out-of-box experience for newcomers and a fast iteration cycle for prototyping are both really important.

I understand that all new code has a maintenance cost, but this should be pretty automatic - just using scripts to repackage the code that's already been written (and I'm doing all that work). Moreover, if a change is so burdensome that it make the starter kit hard to update, maybe it's a hint that it's hard for downstream authors to migrate too.

@@ -0,0 +1,46 @@
{
"name": "jss-starter-kit",
Copy link
Member

Choose a reason for hiding this comment

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

still thinking about the name, other optitons:

  • jss-demo-kit/bundle
  • jss-playground+-(bundle/kit)

Copy link
Member Author

Choose a reason for hiding this comment

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

"starter kit" and "playground" are both known concepts in English. I wouldn't mix them, but if you prefer playground to starter-kit, I can make the change.

Copy link
Member

Choose a reason for hiding this comment

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

I was just brainstorming, if tou think its the best name, lets go for it

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool. I don't have strong feelings on it. jss-starter-kit and jss-playground are both fine for me.

This provides a completely bundled distribution, ready importing from CDNs like unpkg.com for use in sandboxes like Codepen.  It allows this code to work:

```typescript
import { create as createJSS } from 'https://unpkg.com/jss';
```

The PR is a work-in-progress, but I'd rather start a conversation with an example than with prose.  As it is, I can't get `sizeSnapshot` to work with the bundled version.

I also wonder how best to handle plugins/presets.  The easiest options would be to add `unpkg` entry points to each `package`, and force authors to use multiple imports:

```typescript
import { create as createJSS } from 'https://unpkg.com/jss';
import createDefaultJSSPreset from 'https://unpkg.com/jss-preset-default';
```

I wonder what the best way to produce a batteries-included bundle is.  Perhaps there should be another bundle `jss-all` that exports all the other packages.  Then usage would be:

```typescript
import * as jssAll from 'https://unpkg.com/jss-all';

const jss = jssAll.create(jssAll.preset());
```
@kof
Copy link
Member

kof commented Dec 21, 2018

created a new pr, that has this changes backed in + some more

@kof
Copy link
Member

kof commented Dec 21, 2018

lets continue here #941

@appsforartists
Copy link
Member Author

Per @HenriBeck's comments in #941, I wonder if we can split these back into dependent PRs. This seems to be passing except for BrowserStack (which should be irrelevant, since it's a bundling change and not a code one). @kof, do you know why BrowserStack fails?

@kof
Copy link
Member

kof commented Dec 22, 2018

yeah, there is no way to pass the password to browserstack, so essentially its not running any tests, will look into it tomorrow

@appsforartists
Copy link
Member Author

Cool.

I just merged my grammar fix into this PR. If it's a BrowserStack configuration issue, should we merge this PR, and then rebase the rename on top of it?

kof added a commit that referenced this pull request Dec 23, 2018
* Add `unpkg` key to package.json

This provides a completely bundled distribution, ready importing from CDNs like unpkg.com for use in sandboxes like Codepen.  It allows this code to work:

```typescript
import { create as createJSS } from 'https://unpkg.com/jss';
```

The PR is a work-in-progress, but I'd rather start a conversation with an example than with prose.  As it is, I can't get `sizeSnapshot` to work with the bundled version.

I also wonder how best to handle plugins/presets.  The easiest options would be to add `unpkg` entry points to each `package`, and force authors to use multiple imports:

```typescript
import { create as createJSS } from 'https://unpkg.com/jss';
import createDefaultJSSPreset from 'https://unpkg.com/jss-preset-default';
```

I wonder what the best way to produce a batteries-included bundle is.  Perhaps there should be another bundle `jss-all` that exports all the other packages.  Then usage would be:

```typescript
import * as jssAll from 'https://unpkg.com/jss-all';

const jss = jssAll.create(jssAll.preset());
```

* Add jss-starter-kit

* Fixed grammar in warning

* update snapshot
@kof
Copy link
Member

kof commented Dec 23, 2018

merged from #944

@kof kof closed this Dec 23, 2018
@kof
Copy link
Member

kof commented Dec 26, 2018

v10 alpha is published now, so starter kit is also available from unpkg now

@appsforartists
Copy link
Member Author

https://unpkg.com/jss-starter-kit@10.0.0-alpha.3/dist/jss-starter-kit.bundle.js

I see an import * from 'react-jss from the top of the file, which means a browser won't be able to parse that. We may need to explicitly re-export the named exports from react-jss.

@kof
Copy link
Member

kof commented Dec 26, 2018

oh, true

@kof
Copy link
Member

kof commented Dec 26, 2018

and remove that experimental babel plugin

@TrySound
Copy link
Member

@appsforartists How are you gonna use react if you bundle it in this starter kit? React should have only one instance otherwise you get bugs. With this point react is unusable with browser esm without bundling step. I would still recommend to use codesandbox instead and drop unpkg field from it.

@appsforartists
Copy link
Member Author

I haven't used react-jss, so I haven't given this much thought.

My intuition says that jss-starter-kit should probably ship without react-jss. There could be a jss-react-starter-kit that either expects a global variable called React, or inlines a version of React and exports it as React.

@TrySound
Copy link
Member

FYI I fixed that problem with size snapshot plugin in 0.9

bhupinderbola pushed a commit to bhupinderbola/jss that referenced this pull request Sep 17, 2019
* Add `unpkg` key to package.json

This provides a completely bundled distribution, ready importing from CDNs like unpkg.com for use in sandboxes like Codepen.  It allows this code to work:

```typescript
import { create as createJSS } from 'https://unpkg.com/jss';
```

The PR is a work-in-progress, but I'd rather start a conversation with an example than with prose.  As it is, I can't get `sizeSnapshot` to work with the bundled version.

I also wonder how best to handle plugins/presets.  The easiest options would be to add `unpkg` entry points to each `package`, and force authors to use multiple imports:

```typescript
import { create as createJSS } from 'https://unpkg.com/jss';
import createDefaultJSSPreset from 'https://unpkg.com/jss-preset-default';
```

I wonder what the best way to produce a batteries-included bundle is.  Perhaps there should be another bundle `jss-all` that exports all the other packages.  Then usage would be:

```typescript
import * as jssAll from 'https://unpkg.com/jss-all';

const jss = jssAll.create(jssAll.preset());
```

* Add jss-starter-kit

* Fixed grammar in warning

* update snapshot
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.

None yet

4 participants