vazco / universe-modules

Use ES6 / ES2015 modules in Meteor with SystemJS
https://atmospherejs.com/universe/modules
MIT License
52 stars 7 forks source link

Version 0.3.0 seems to not respect System map configuration #9

Closed tel closed 9 years ago

tel commented 9 years ago

Right now I have a bunch of modules with System.config setups like the following:

System.config({
  map: {
    "reify:immutable": "reify:immutable/index",
  },
});

The goal of course is to simulate npm style a bit. Unfortunately these appear not to be respected any longer.

MacRusher commented 9 years ago

This may have to do something with changes in normalization or prefixing every module with meteor://. Of course this should work like before. I will try to fix the regression tomorrow.

PS. I think that SystemJS package API will be better fit for configuring exports from meteor packages. map should work still, but I'll be endorsing using packages for that purpose soon. Just a heads up :)

tel commented 9 years ago

Oh, cool. That is better. I'll move to it :+1:

ghost commented 9 years ago

I also have this issue with 0.3. Here is a small Meteor app where the issue can be reproduced: https://github.com/Sanjo/meteor-es2015-modules-and-di

The boot.js contains the mapping. I also get an error with loading relative paths. Not sure if that would go away when the mapping is fixed.

If you use my di branch of universe:modules that is based on 0.2.1, it works.

MacRusher commented 9 years ago

I have some problems doing this "the right way", and I really don't want to introduce temporary workarounds, minimizing backward compatibility issues is important for me.

I'm not sure if the whole meteor:// thing was a good idea, it solves some problems but introduced new ones like this.

I will take a look at your use case and try to solve it, keeping api from 0.2.1

ghost commented 9 years ago

I'm not sure if the whole meteor:// thing was a good idea, it solves some problems but introduced new ones like this.

Just out of interest. What problems did it solve?

tel commented 9 years ago

It was introduced, I think, primarily to fix #5. The thread there is a bit meandering, but ultimately it came down to a difference in normalization behavior between module loader polyfills in different browsers.

MacRusher commented 9 years ago

SystemJS is designed to work on URLs, so if you register a module foo/bar it actually will be something like http://hostname/foo/bar.

Base URL is detected automatically or can be set manually.

There are few problems with this: there will be completely different full module names on client and server (http://hostname/foo/bar vs file:///path/to/your/meteor/server/directory/foo/bar) and you actually can live with that, but real problems starts when you're using modules like author:package/foo. SystemJS don't like the colon and converts this to author://package/foo and everything starts to fail. There could be also other corner cases, like different browsers tend to convert urls in different ways (this one was fixed with new SystemJS release and forcing polyfill instead of native implementation).

My idea to fix this issue was to introduce own "protocol" that we can have full control over. That is why in 0.3 every module (regardless client/server) was registered as meteor://foo/bar or meteor://author:package/foo and loader was adjusted to work with this kind of namespaces.

But my thinking now is that this is a dead end. This can cause problems, e.g when we introduce fetching modules on demand instead bundling them. This also make SysyemJS configuration (like map or paths) more difficult.

Right now I'm testing few solutions for 0.4, but I'll probably drop meteor:// and focus on better handling package names.

If you have some thought on this issue I'm open to suggestions :)

ghost commented 9 years ago

but real problems starts when you're using modules like author:package/foo. SystemJS don't like the colon and converts this to author://package/foo and everything starts to fail.

I think a good solution is to normalize package names by replacing the colon with a slash. sanjo:foo becomes sanjo/foo. I have implemented that here: https://github.com/Sanjo/meteor-es2015-modules-and-di/blob/master/boot.js#L1-L21. It registers all packages as modules.

ghost commented 9 years ago

Another thing that maybe makes sense is to normalize the package name sanjo:foo to {sanjo/foo}. This curly braces will be used for less and stylus to load files from other packages. See https://github.com/meteor/meteor/issues/4800

The less and stylus packages now support cross-package imports! If you specify a file like @import "{package}/file.js" (with the curly braces), it will load the file from that package. Use @import "{}/file.js" to load a file from your app.

Could make sense for the edge case where you want to import actually a file instead of the package. Let's say there is a file sanjo/foo but also the package sanjo:foo. Then you could not load the file with my previously proposed convention.

MacRusher commented 9 years ago

Another thing that maybe makes sense is to normalize the package name sanjo:foo to {sanjo/foo}. This curly braces will be used for less and stylus to load files from other packages. See meteor/meteor#4800

This is really good idea, thanks for the tip! If MDG introduces new syntax for importing from packages, we should definitely follow it instead of inventing our own. They introduced it today, so really good timing :)

Right now there is a working version of modules on the devel branch that uses _ instead of : in package names (the same trick that meteor uses to be compatible with windows paths). I'll make it compatible with introduced less/stylus syntax.

I'll also take a look at new build plugin api and see if modules could benefit from it.

MacRusher commented 9 years ago

I just released new version - 0.4.0. You can see all changes in changelog.md

meteor:// prefix was dropped, so if you were using it you'll have to update your code.

Right now importing from package requires new syntax compatible with less/stylus importing syntax that will be introduced in 1.2: https://github.com/meteor/meteor/issues/4800

I've provided backward compatibility, and you'll see a warning on imports that should be fixed.

About this issue: SystemJS map/packages etc will work, but you have to provide normalized module names as a params (this is SystemJS thing).

You should never hardcode normalized name (this is a internal value and can change from version to version), instead you can use System.normalizeSync. More info in the readme: https://github.com/vazco/universe-modules#systemjs-api

I'm closing this issue, but feel free to post new issues if something is not working for you.

tel commented 9 years ago

This looks excellent! (Although now I need another module import refactor phase ;))

:+1: :+1: :+1: :+1: :+1:

On the other side of this? Do you have any further ideas about how to use System.config({packages: ...}) yet?

MacRusher commented 9 years ago

Although now I need another module import refactor phase ;)

Yeah sorry for that, I'm trying to keep backward compatibility as much as possible but this could happen also in the future until we're at 1.0.0.

On the other side of this? Do you have any further ideas about how to use System.config({packages: ...}) yet?

There is example usage in the docs: https://github.com/vazco/universe-modules#systemjs-packages Basically you need to declare some global name for the package (like bootstrap in the example) and set is to own package path. Then you can set default file and some other options that will affect only your package (mappings etc). Its working really nice for me, but there could be some weird edge cases I didn't find yet :)

tel commented 9 years ago

Hey! Getting genuine modules into Meteor is worth the pain 100-fold. This library helped me to untwist some fairly remarkable package trickery I was doing to get halfway decent namespaces... along with all those 0namespace.js files people often resort to.

And if packages is stable under the current version then I'll experiment a bit with it. There's a lot of flexibility in JS modules... determining a halfway sane style is actually pretty non-trivial!