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

Create can-define-list and can-define-map #307

Open
2 of 4 tasks
chasenlehara opened this issue Jan 19, 2018 · 3 comments
Open
2 of 4 tasks

Create can-define-list and can-define-map #307

chasenlehara opened this issue Jan 19, 2018 · 3 comments
Labels

Comments

@chasenlehara
Copy link
Member

chasenlehara commented Jan 19, 2018

As a user, learning about and typing can-define/list/list & can-define/map/map feels redundant. Additionally, the documented package in the canjs.com sidebar is can-define, which is a lower-level API that people mostly don’t use.

  • Create can-define-list and can-define-map packages
  • Move can-define to the Infrastructure collection (as of 2019-12-09 can-define is in the legacy collection)
  • Write a codemod for switching from can-define to the individual packages
  • Integrate the updated packages into the main can package
@phillipskevin
Copy link
Contributor

phillipskevin commented Jan 29, 2018

So, I started working on this and released [email protected] and [email protected]. In doing this... a few problems came up. I'm going to detail those here so we can decide what to do.

Issues

can-define dependency issue

Both can-define-map and can-define-list depend on can-define. They're actually basically just aliases, with code that looks like:

require("can-define/list/list");
module.exports = require("can-define/map/map");

Right now, these dependencies look like "can-define": "^2.0.0", which means that if someone wants to lock the dependencies in their app, it won't work.

Someone that has this dependency in their package.json:

"can-define-map": "1.0.0"

will still get new versions of can-define, where the source code really is.

One solution to this would be to lock the dependency of can-define so that the versions of can-define-map and can-define-list always match can-define's version.

This has some development overhead because we'll need to always remember to release all three packages whenever we release can-define. The bigger problem is that this could lead to users having multiple versions of can-define. For example, if we release [email protected] and someone only updates their version of can-define-map, they could end up with dependencies like

This could create really weird scenarios where two lists created differently use different versions of can-define:

import DefineMap from "can-define-map";
import DefineList from "can-define-list";

const map = new DefineMap({
  listOne: [] // This is using [email protected]'s version of can-define/list/list
});

const listTwo = new DefineList([]);  // This is using [email protected]'s version of can-define/list/list

can-define docs issue

There is also an issue that the docs for can-define/map/map are in the can-define-map package, so if you update some code, you'll have to update the docs in a separate repo.

If we decide to go ahead with can-define-map/can-define-list, we should just move the docs back and add a note somewhere that the @module name doesn't match the actual module name.

What should we do instead

Instead of creating separate packages, we could make it so you can import can-define and get DefineMap and DefineList. Something like:

import define from `can-define`

Foo = define.Map.extend({
  
});

This is uglier, but doesn't have all of the weird problems that can-define-map/can-define-list create. It's also more in line with what we're doing with can-observe (observe.Object, observe.Array).

Anyone have other ideas?

@andrejewski
Copy link
Contributor

Why don't we add can-define/map/index.js? So people can get can-define/map? I think the map/map is conceptually the harder problem, not that can-define has submodules.

@mjstahl
Copy link
Contributor

mjstahl commented Nov 19, 2018

This is uglier, but doesn't have all of the weird problems that can-define-map/can-define-list create. It's also more in line with what we're doing with can-observe (observe.Object, observe.Array).

This would be my preferred method as I could do something like:

import { Map as DefineMap } from 'can-define';

or you could just export the object as DefineMap so that Users can just:

import { DefineMap } from 'can-define';

This looks a little redundant when you don't destructure.

import define from 'can-define';
Foo = define.DefineMap.extend({ });

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

No branches or pull requests

4 participants