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

Use pascal case for global namespace #873

Merged
merged 4 commits into from Oct 1, 2018
Merged

Use pascal case for global namespace #873

merged 4 commits into from Oct 1, 2018

Conversation

kof
Copy link
Member

@kof kof commented Oct 1, 2018

No description provided.

@kof kof requested a review from TrySound October 1, 2018 10:29
@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

This will make jss be accessed with Jss, ReactJSS, JssPluginCache etc. Is it okay?

@kof
Copy link
Member Author

kof commented Oct 1, 2018

Damn, checked old exports from jss

(function (global, factory) {
  typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
  typeof define === 'function' && define.amd ? define(['exports'], factory) :
  (factory((global.jss = {})));
}(this, (function (exports) { 'use strict';

@kof
Copy link
Member Author

kof commented Oct 1, 2018

Also react-jss was using "reactJss"

	if(typeof exports === 'object' && typeof module === 'object')
		module.exports = factory(require("React"));
	else if(typeof define === 'function' && define.amd)
		define(["React"], factory);
	else if(typeof exports === 'object')
		exports["reactJss"] = factory(require("React"));
	else
		root["reactJss"] = factory(root["React"]);

@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

I think it's okay for breaking change. Let's go with it?

@kof
Copy link
Member Author

kof commented Oct 1, 2018

Its just confusing that Jss is a namespace but its not a constructor, I think React isn't either.

@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

Then we need to provide an explicit globals object. And convert name with normal camel case.

@kof
Copy link
Member Author

kof commented Oct 1, 2018

I wonder when will be the next time we have same problem with some peer, we can't actually decide globally what global namespace any of the peer dependencies may use.

@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

We can actually. Just take a look at peer umd
https://unpkg.com/react@16.5.2/umd/react.development.js

@kof
Copy link
Member Author

kof commented Oct 1, 2018

updated to use for jss packages camel case, but for peers - pascal case

@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

jss is also peer dependency, isn't it?

@HenriBeck
Copy link
Member

HenriBeck commented Oct 1, 2018 via email

@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

Should be on react-jss too.

@kof
Copy link
Member Author

kof commented Oct 1, 2018

Shit :)

@kof
Copy link
Member Author

kof commented Oct 1, 2018

Any ideas how to do this without maintaining a whitelist?

@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

No way. Lib authors may not follow only one pattern.

@TrySound
Copy link
Member

TrySound commented Oct 1, 2018

It's not so big list. react, react-dom and jss. Am I miss something?

@kof
Copy link
Member Author

kof commented Oct 1, 2018

Ok, at least we should build a warning when there is an unknown peer dependency.

@kof
Copy link
Member Author

kof commented Oct 1, 2018

Ok, whitelist with a warning it is now

@kof kof merged commit 92accb2 into master Oct 1, 2018
@TrySound TrySound deleted the fix-global-namspace branch October 1, 2018 17:22
bhupinderbola pushed a commit to bhupinderbola/jss that referenced this pull request Sep 17, 2019
* use pascal case for global namespace

* docs for new dist files

* use camel case without pascal for jss packages globals

* whitelist for peer deps and warning
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

3 participants