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

Provide JS Modules export? #50

Closed
developit opened this issue Sep 2, 2018 · 8 comments
Closed

Provide JS Modules export? #50

developit opened this issue Sep 2, 2018 · 8 comments

Comments

@developit
Copy link

developit commented Sep 2, 2018

Hi @sindresorhus!

This module is pretty popular, and its functionality is fairly segmented (get vs set). I'm willing to bet a decent number of people are only using this to safely retrieve the value at a dotted path (not set).

Given that assumption, I was wondering if you'd be open to providing a JS Modules version where { set, get } are named exports? I'd be happy to open a PR, though I'd need to know what solution you'd prefer for transpiling modules to commonjs (I'm biased).

Cheers!

@sindresorhus
Copy link
Owner

I'm generally not interested in having a transpile-step in my modules, but I've started appreciating TypeScript, which actually makes a compile-step worth it. So what I'm saying is that if anyone rewrites this module in TS, I'm willing to add a compile-step. Happy to use https://github.com/developit/microbundle for that.

@rand0me
Copy link

rand0me commented Sep 26, 2018

Hi!
I wanted to participate in this issue, but found that this module exports .delete() method, which can't be exported as named exports (because delete is a keyword in JS). My decision is to rename this method to remove, but leave it as delete in CommonJS exports.

Can you suggest a better solution for this?

@sindresorhus
Copy link
Owner

I'm having doubts whether these makes sense as named imports. get as in import {get} from 'dot-prop; is not very readable in code if it's far from the import statement, and import {get as getProperty} from 'dot-prop'; is quit verbose...

@sindresorhus
Copy link
Owner

I wanted to participate in this issue, but found that this module exports .delete() method, which can't be exported as named exports (because delete is a keyword in JS).

I think del or delete_ would be better. I prefer the latter and it's what I've been using in many other places for reserved keywords.

@sindresorhus
Copy link
Owner

If they are going to be "named exports", I think it would make sense to make them named something like import {getProperty, deleteProperty} from 'dot-props';. Thoughts?

@jeremyben
Copy link

Your package already has type definitions which reflect the simple named import :
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/dot-prop/dot-prop-tests.ts

I suggest keeping it that way, i.e. concise instead of verbose. I know everyone has a different opinion on this naming dilemna.

Another point for me is don't fix what is not broken. Ask yourself : did somebody complain about the concise naming ?

Finally, you already have some kind of "standard" on this matter : Lodash also exports a simple get or set.

@saibotsivad
Copy link

Just another perspective: I actually use the import { get } from 'dot-prop' a lot in a large API project, and it seems pretty readable to me. An example route handler might look like:

import { get } from 'dot-prop'

export default async ({ mediator, query, security }) => {
    if (get(security, 'security/public-mobile-app')) {
        query.state = 'published'
    }

    return {
        status: 200,
        body: {
            data: await mediator.call('controller/players/find', { query })
        }
    }
}

Coming from projects that use lodash with the _ a lot, I feel like _.get( is just as readable as get( but I am all for making names more wordy:

import { getProperty } from 'dot-prop'

export default async ({ mediator, query, security }) => {
    if (getProperty(security, 'security/public-mobile-app')) {

@sindresorhus
Copy link
Owner

sindresorhus commented Jan 21, 2022

import {getProperty, deleteProperty} from 'dot-props';.

I'm going with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants