can-define icon indicating copy to clipboard operation
can-define copied to clipboard

Create can-define-list and can-define-map

Open chasenlehara opened this issue 7 years ago • 3 comments

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.

  • [x] Create can-define-list and can-define-map packages
  • [x] 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

chasenlehara avatar Jan 19 '18 15:01 chasenlehara

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

[email protected]
-- [email protected]
[email protected]
-- [email protected]

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?

phillipskevin avatar Jan 29 '18 16:01 phillipskevin

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.

andrejewski avatar Jan 29 '18 17:01 andrejewski

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({ });

mjstahl avatar Nov 19 '18 15:11 mjstahl