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

export mjs for esm #37

Open
xiaoxiangmoe opened this issue Jun 19, 2020 · 20 comments · May be fixed by #38
Open

export mjs for esm #37

xiaoxiangmoe opened this issue Jun 19, 2020 · 20 comments · May be fixed by #38

Comments

@xiaoxiangmoe
Copy link

https://nodejs.org/api/esm.html#esm_conditional_exports

Add support for esm.

If you think this proposal is reasonable, I'll create a PR for this.

@nbubna
Copy link
Owner

nbubna commented Jun 19, 2020

A PR for this would be great!

@xiaoxiangmoe
Copy link
Author

xiaoxiangmoe commented Jun 19, 2020

Case/src/Case.js

Lines 169 to 173 in 7f054b0

// export Case (AMD, commonjs, or global)
var define = typeof define === "function" ? define : function(){};
define(typeof module === "object" && module.exports ? module.exports = Case : this.Case = Case);
}).call(this);

Can we change the source code to esm and package it into umd and esm?

@nbubna
Copy link
Owner

nbubna commented Jun 19, 2020

I'm not very familiar with Rollup, but if it seems simple enough, i'm willing to try it out.

@nbubna
Copy link
Owner

nbubna commented Jun 19, 2020

And yes, changing the source to ESM makes sense, as long as we still produce UMD for compatibility.

@xiaoxiangmoe
Copy link
Author

I can provide a PR containing umd, esm with rollup.This will not be very complicated.

@xiaoxiangmoe
Copy link
Author

I am a vite user.
Currently, vite only supports esm, which drives us to provide esm support to all community libraries.

@xiaoxiangmoe xiaoxiangmoe linked a pull request Jun 19, 2020 that will close this issue
@xiaoxiangmoe
Copy link
Author

@nbubna Please review it.

TODO: simplify codes to support tree shaking for downstream users.

@nbubna
Copy link
Owner

nbubna commented Jun 19, 2020

I will! Have patience and get some sleep. ;)

@nbubna
Copy link
Owner

nbubna commented Jun 19, 2020

I'm pondering making this a major version bump. Then i can remove squish, Component support, possibly even trim some fat by using ES6 features, and maybe drop Grunt for Rollup entirely. It is a little odd to be maintaining two build systems, and Grunt is largely stale. Thoughts?

@nbubna
Copy link
Owner

nbubna commented Jun 19, 2020

I'm also thinking of getting rid of toCase, as i don't believe it gets much use (i don't use) and don't like modifying String.prototype.

Since you are more experienced with Rollup, do you think you can get it to run the qunit tests? I that would be the most important piece needed.

@xiaoxiangmoe
Copy link
Author

xiaoxiangmoe commented Jun 20, 2020

Case.type('bang', function(s) {
     return Case.upper(s, '!')+'!';
});

This usage is very dynamic, esm can only be exported statically.
Static export can make rollup and webpack tree shaking more convenient.
Do you have any ideas?

@nbubna
Copy link
Owner

nbubna commented Jun 20, 2020

Moving to jest is an excellent idea. As for the dynamic case creation, the idea is that people would export\import the Case object if they wish to add their own case types.

@xiaoxiangmoe
Copy link
Author

xiaoxiangmoe commented Jun 20, 2020

If we continue to use esm, it is difficult for us to achieve API compatibility, because esm can only be exported statically. Maybe we can define an object named case and export default Case;

@xiaoxiangmoe
Copy link
Author

import * as _ from './utils.mjs';
import * as types from './types.mjs';

const typesArray = Object.keys(types);

function of(s) {
    for (var i = 0, m = typesArray.length; i < m; i++) {
        if (Case[typesArray[i]].apply(Case, arguments) === s) {
            return typesArray[i];
        }
    }
}

function flip(s) {
    return s.replace(/\w/g, function (l) {
        return (l == _.up.call(l) ? _.low : _.up).call(l);
    });
}
function random(s) {
    return s.replace(/\w/g, function (l) {
        return (Math.round(Math.random()) ? _.up : _.low).call(l);
    });
}

function type(type, fn) {
    Case[type] = fn;
    typesArray.push(type);
}

const Case = {
    _:{
        ..._,
        types: typesArray,
    },
    of,
    flip,
    random,
    type,
    ...types,
}

export default Case;

If it is to be compatible, I can only write in this form, which looks strange, and it is not convenient for tree-shaking.

@nbubna
Copy link
Owner

nbubna commented Jun 20, 2020

It should export Case as the default exports and the functions as named exports. That should be fine for tree-shaking. I'm off to bed and have a busy weekend. I may have limited time to review this before Monday.

@xiaoxiangmoe
Copy link
Author

// TODO: can't export these
// export {of,flip,random,type,utils as _ };
// export * from './types.mjs';

export default Case;

If we export default Case with export other functions. In umd, after Case.type('bang',...) We can only get Case.default.bang. Case.bang is undefined.

@nbubna
Copy link
Owner

nbubna commented Jun 20, 2020

Well, that's not a very helpful UMD impl. Since we're talking about bumping major version anyway, i think we may have to give up the Case[customType] part of the API and just have Case.type('bang', fn) register the function for Case.of to use and return the function for the user. I don't like that, but i'll have to think and look at this more to decide. I honestly don't think registering custom case types on the Case object is that high value, though.

@xiaoxiangmoe
Copy link
Author

xiaoxiangmoe commented Jun 20, 2020

The static nature of esm makes it inconvenient for us to dynamically inject on the module.
I also think Case.of and Case.type are not particularly useful.

@nbubna
Copy link
Owner

nbubna commented Jun 20, 2020

I actually use Case.of myself, so that is certainly useful enough to keep, IMO. I have made occasional use of custom case types, but i do not specifically need to be able to do

Case.type('bang', fn);
Case.bang(text);

So, i think i am willing to change Case.type to something like Case.register(name, fn) to be more clear that it simply registers a type for use with Case.of, but does not add the new type to the Case object itself.

@xiaoxiangmoe
Copy link
Author

That sounds achievable.

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 a pull request may close this issue.

2 participants