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

problem with dots in filenames #756

Closed
johnpapa opened this issue Sep 3, 2015 · 29 comments
Closed

problem with dots in filenames #756

johnpapa opened this issue Sep 3, 2015 · 29 comments

Comments

@johnpapa
Copy link

johnpapa commented Sep 3, 2015

The code below works fine for mapping all imported modules in y app folder to *.js. However, if the file name has a dot in it, it does not work. Appears to be a bug. Or is there a better way to solve it?

System.config({
  packages: {
    'app': {
      defaultExtension: 'js'
    }
  }
});

import {FooComponent} from './foo.component'; tries to load with no js extension, so it gets a 404. If I renamed to ./foo-component then it works

@guybedford
Copy link
Member

This is actually by design, but I see your point since dots in filenames are relatively common.

Your best bet in these scenarios is to create a meta or map config:

System.config({
  packages: {
    'app': {
      meta: { '*.component': {} }
    }
  }
});

Update: Sorry this only actually applies to the case where you don't want any extension added, not where you do want an extension added.

@guybedford
Copy link
Member

So to summarize here, the way to deal with these today is as individual map exceptions, and that was the way the design was made to handle this originally:

System.config({
  packages: {
    'app': {
      defaultExtension: 'js',
      map: {
        './foo.component': './foo.component.js'
      }
    }
  }
});

I know this is not ideal though listing each and every filename with a dot though, so perhaps we should consider something like the meta example above for handling these cases.

The conflict here is that it is more useful not to impose only a single extension for the package to allow loading other asset types with plugins like .css etc, so that only adding an extension when there wasn't already an extension seemed the simplest solution.

@johnpapa
Copy link
Author

johnpapa commented Sep 3, 2015

Thanks for the fast reply.

It is unrealistic to list all files, but a glob pattern could be used. It seems like a lot of friction to have to do all of this to use transpiled js modules (from babel or typescript). for example being able to say import {Foo} from './foo' is ideal. foo.ts is what we want referenced but foo.js is what we want loaded by the module loader

I could have dots ,dashes, or multiple of each in a file name.

Right now my folder structure for Angular 2 looks like this:

node_nodules/
src/
    index.html
    config.js //for systemjs
    app/
        app.ts
        customer.component.ts
        customer-detail.component.ts
        customer.service.ts

I just tried this config but it did not map to the components nor services, and I would expect some sort of starring allowance:

System.config({
  packages: {
    'app': {
      map: {
        './*.component': './*.component.js',
        './*.service': './*.service.js'
      },
      defaultExtension: 'js'
    }
  }
});

but instead I have to do this

System.config({
  packages: {
    'app': {
      map: {
        './customer.component': './customer.component.js',
        './customer-detail.component': './customer-detail.component.js',
        './customer.service': './customer.service.js'
      },
      defaultExtension: 'js'
    }
  }
});

@johnpapa
Copy link
Author

johnpapa commented Sep 3, 2015

BTW - the point being that in a small app I already have 10 of these maps listed. For a larger app (which I have many of) with 100's and 1000's of files, this is not maintainable.

The real goal is to have some way to be able to allow importing of the most common use cases, which are 3rd party js and importing your own javascript. I'm open to any suggestions and hoping I am just missing something very obvious here :)

@guybedford
Copy link
Member

Sure, it isn't ideal and the assumption was always that dots in filenames would be rare enough to work with individual exceptions. It looks like this is an oversight though, so let me see if I can come up with something for this.

@johnpapa
Copy link
Author

johnpapa commented Sep 3, 2015

I appreciate it. I'm working on Angular 2 material for helping folks get started, to give some context.

@guybedford
Copy link
Member

Ok I've managed to rework the package config to support the following:

System.config({
  packages: {
    app: {
      defaultExtension: 'js',
      main: 'app.ts', // will load "app.ts"
      meta: { '*.ts': { loader: 'typescript' } },
      map: {
        './x': './y' // will load "./y.js"
      }
    }
  }
});

Where System.import('app/some.file') will load app/some.file.js and System.import('app/another.file.ts') will load app/another.file.ts.

This is strictly a breaking change in SystemJS, so I'll have to post this out as a 0.19 shortly.

@johnpapa
Copy link
Author

johnpapa commented Sep 3, 2015

Thanks.

If I transpile on the server, i dont need the loader to specified, right? I dont want transpiling in the browser due to speed

Also, I'm curious ... the way I would think module loading would work is this in progressive order:

  1. check if the module name given exactly matches something registered
  2. if not ... then check of the module path exists and register it
  3. if not ... then check if the module path exists using a default file extension
  4. if not .... then check if the module path exists using mappings

This would allow us to have import of angular2/angular2 and not have it run int any attempts to match a file path or extension. It would also provide for paths in the import and the 95% use case where paths can default to .js (or a configurable extension). And it stil allows for json, css, etc.

@guybedford
Copy link
Member

Sure, no problem - this was a good point and I think it simplifies the workflows in a more obvious way without adding too much complexity to the package configuration.

Transpiling in the browser is always optional, and cache workflows will develop around this via development servers and in-browser caching techniques in future.

Precompilation is certainly encouraged but in the case of the above the defaultExtension would need to match in these changes, otherwise precompiled TypeScript would be loaded as .ts.js if the defaultExtension was js. It's probably advisable to output transpiled typescript as js files though.

  1. check if the module name given exactly matches something registered

The loader registry contains currently loaded modules keyed by URL, that would effectively be this step.

  1. if not ... then check of the module path exists and register it

This would be if you've loaded a bundle into the page then bundles get checked next, which are registered and executed only when imported.

  1. if not ... then check if the module path exists using a default file extension

This defaultExtension stuff is integrated into the normalization algorithm alongside using package map and meta configs.

  1. if not .... then check if the module path exists using mappings

The mapping algorithms are always applied to all module loads, even for existing modules, as it is part of the normalization. It can be worth running some tests with System.normalize('a', 'b').then(console.log.bind(console)) to see how it works against the configurations.

Can you clarify what you mean about having an import to angular2/angular2 not needing to match file paths or extensions? Do you mean you'd just like to have angular2/angular2 as the direct name in the registry?

@johnpapa
Copy link
Author

johnpapa commented Sep 3, 2015

On the last point, let me try to lay out the situation where this was odd for me. Let me explain some of the things I found through this journey.

Let's say we have a project where everything is in the root. Something simple.

node_modules/
    angular2/
    systemjs/
app.component.ts
foo.component.ts
bar.component.ts
data.service.ts
index.html
package.json
tsconfig.json
tsd.json

This type of structure, while not ideal, causes some issues when importing with the default SystemJS settings. For example:

import {View} from 'angular2/angular2';
import {foo} from './Foo';

This is what you were asking about above
If we use default JS extensions, we cover the foo but the app fails with a 404 as it tries to load angular2/angular2.js This is one reason I used the folder structure I chose.

Even If I go to the following structure, the code has issues. Now we have a level of separation of the 3rd party modules due to all source under its own folder, but there is difficulty in creating a mapping since the code is in the same level as the index.html (which contains the System.config).

node_modules/
    angular2/
    systemjs/
src/
    app.component.ts
    foo.component.ts
    bar.component.ts
    data.service.ts
    index.html
    tsconfig.json
    tsd.json
package.json

Which is what lead me to the map section in SystemJS and the named folder for the custom source code, thus creating:

node_modules/
    angular2/
    systemjs/
src/
    app/
        app.component.ts
        foo.component.ts
        bar.component.ts
        data.service.ts
    index.html
    tsconfig.json
    tsd.json
package.json

This allows the node_modules to be loaded without conflict and the source in app to be mapped and is effectively what I showed earlier in this chain.

@guybedford
Copy link
Member

Firstly note that defaultJSExtensions is a property that will be deprecated in future.

For a package structure like your example of:

node_modules/
    angular2/
    systemjs/
app.component.ts
foo.component.ts
bar.component.ts
data.service.ts
index.html
package.json
tsconfig.json
tsd.json

The suggested package configuration in SystemJS would be something like:

System.config({
  map: {
    angular: 'node_modules/angular2'
  },
  packages: {
    '.': {
      defaultExtension: 'ts'
    },
    'angular': {
      defaultExtension: 'js',
      main: 'index.js'
      // ... etc package configuration ...
    }
  }
});

The same packages configuration system can be adapted to both the second and third folder structures as well, so that the choice of folder structure is very much up to your own project needs.

Package configuration is really the focus for configuration in the latest SystemJS. Let me know if this makes sense or if there are any gaps in the use case you want to discuss further at all.

@johnpapa
Copy link
Author

johnpapa commented Sep 4, 2015

It makes sense. You've been very helpful and I appreciate it. I feel that the config is very verbose for simple scenarios. Consider someone getting started with some of these frameworks. Using SystemJS would appear to them, but the ideal situation would be to have basic scenarios work out of the box.

First choice woul dbe to have SystemJS default some of these behaviors.

Second choice would be to have these frameworks, like Angular2, come out of the box with a default configuration for SystemJS that the user could then override. For example, would Angular2 set this config up (what you showed above) and let folks override it?

@guybedford
Copy link
Member

Sure, no problem at all!

Web workflows today always do require some boilerplate. Project scaffolding tools can be useful here. jspm is also a project for SystemJS that aims to give the full out-of-the-box experience.

That said, the newest feature in SystemJS is exactly about this configuration problem, and you've just inspired me to make a couple of changes here as well.

With this feature, instead of having to write:

System.config({
  map: {
    angular: 'node_modules/angular2'
  },
  packages: {
    '.': {
      defaultExtension: 'ts'
    },
    'angular': {
      defaultExtension: 'js',
      main: 'index.js'
      // ... etc package configuration ...
    }
  }
});

One can store the configuration in the package.json file within Angular itself, and only write:

System.config({
  map: {
    angular: 'node_modules/angular2'
  },
  packageConfigPaths: ['node_modules/*/package.json'],
  packages: {
    '.': {
      defaultExtension: 'ts'
    }
  }
});

That is, the loader would dynamically request the config by loading the package.json file for Angular. The wildcard could also be omitted for the full angular package config path here.

In this way, the SystemJS package configuration then becomes a package.json convention.

This is very new still, and not released yet, so would be interested to hear what you think.

@johnpapa
Copy link
Author

johnpapa commented Sep 4, 2015

I like it. angular and others can add a section the their package.json for systems config

Raises a few questions

Could that be the default behavior and system js just checks there?

How would modules with conflicting settings be resolved?

What would that look like for angular?

And of course how soon can we have the dots feature.? Lol

@johnpapa
Copy link
Author

johnpapa commented Sep 4, 2015

BTW - No convinced yet on JSPM. The amount of stuff it adds to configuration ... wow. Separate conversation on that.

@guybedford
Copy link
Member

Glad to hear you like the idea - I've also added the ability for the package config to be prefixed under a "systemjs" property. That effectively encourages a SystemJS package.json convention.

Could that be the default behavior and system js just checks there?

Yes, this does require each user individually adding:

System.config({
  packageConfigPaths: ['node_modules/*/package.json', 'bower_components/*/bower.json'] // etc
});

We do need this information because it tells us where to find node_modules, which is not something that can be assumed.

How would modules with conflicting settings be resolved?

Not sure what you mean by conflicting settings here, but does the "systemjs" package.json prefix answer that? Also note that local configuration merges into the package config so I can add say:

System.config({
  map: {
    underscore: './local/underscore.js'
  },
  packages: {
    'angular': {
      map: { lodash: 'underscore' }
    }
  }
});

Or whatever, which will apply that map, even if Angular has a map for lodash already in its package config.

What would that look like for angular?

I've only really thought about this idea of a "systemjs" package property today, and it should be perfectly compatible with the jspm approach as well, but still, important conventions should be considered carefully - will certainly post my thoughts further and would value feedback.

And of course how soon can we have the dots feature.? Lol

It's looking good to go, but because this is a major release, I need to make sure the edge cases are all working out ok as it would be annoying to have to immediately post out a 0.20, so it's not something that can be rushed unfortunately. but it is the top priority.

@johnpapa
Copy link
Author

johnpapa commented Sep 5, 2015

I'd like to travel down that road a bit with the angular and traceur (for now) needs in my app. The config gets larger and larger then as we have to optionally configure those to use .js and the source to use .ts. i tried many variations but it got pretty convoluted so I gave up and went back to the folder structure to reduce the config.

I tried this with this repo ... https://github.com/johnpapa/angular2in22-docs and opted for the simpler approach.

@guybedford
Copy link
Member

@johnpapa sure, I think that sounds good. Please do keep me posted with how you get on with things. Also, with this new handling of defaultExtension, I've added #759 to make defaultExtension: 'js' the default for all packages which should make things easier too. On a slightly related note - I notice that the Angular build is a SystemJS bundle and not an SFX bundle, which is probably not ideal, and leading to issues like #761 which sounds very similar to your issue mentioned previously.

SystemJS named bundles are recommended to be created and managed within the environment they are run in, and not distributed to other environments, as the exact module naming system is unique to the target consumer environment. Using named bundles as a distribution format leads to these problems and also stops multi-version from working as module names will never always match up.

I'd strongly advise distributing Angular as an SFX bundle if possible here (cc @robwormald).

@johnpapa
Copy link
Author

johnpapa commented Sep 5, 2015

Thanks.

There is a SFX bundle in the angular bundles folder. From what I gathered, that was just to include systemjs and/or traceur in the bundle. if I use that, would I need SystemJS? What other benefits does it have?

Not quite sure I understand the difference in named or not. Is this outlined somewhere? This just listes the loaders https://github.com/systemjs/builder#sfx-format

@robwormald
Copy link

@guybedford my understanding on the (alpha) distribution process is that the bundles exist mainly for easy debugging of stuff (mainly on plunker) while ng2 is still in development. The long term plan is SFX for es5 users and npm/jspm for es6/TS users.

@johnpapa the main difference is that an SFX bundle self-executes and in the case of ng2, adds itself to window.ng as soon as it's included. Non-SFX bundles simply populate the module cache but don't actually do anything until they are imported. You can see the SFX hook here https://github.com/angular/angular/blob/dfa5103b1d390ee9744548ca4fe08bfa73ce2dd5/modules/angular2/angular2_sfx.ts

SFX bundles usually bundle a micro loader and as such are the closest analogue to "regular" es5 code. You can just drop in the file in a script tag and go.

@robwormald
Copy link

Fwiw, the pattern I've found that works reasonably well with the non-SFX bundles is declaring a "app" package that uses a default TS extension, as seen here http://embed.plnkr.co/wj4lRtAwNMry8ToNzLrm/preview

In my experience having dots in filenames is a gigantic nightmare and I avoid it at all costs :-D

@johnpapa
Copy link
Author

johnpapa commented Sep 5, 2015

RE: dots ... dots is only a nightmare if libraries do not support it. :) I've been using dots very successfully for years in apps. its just a character and the library should support it (and looks like this one will, thank you!) dots actually make things like pattern matching and globs easier, IMO. Not to mention readability.

I look forward to testing this out, thanks

@guybedford
Copy link
Member

@robwormald SFX bundles for all-ES6 modules will be getting an optimization in future to skip the microloader implementation. Basically we only need this when bundling multiple module formats together. SFX bundles also now support exclusions and output as custom module formats so that I can create an SFX bundle that is itself an ES6 module. In this way, they've kind of morphed into a build over an sfx-bundle, and may be rebranded at some point when this work is completely stable. Building and bundling then become the two modular folding operations. The only difference being building loses track of the internal module registry, and can do dead-code optimizations (ala static builds used everywhere today), while bundling maintains this registry and can't do dead-code optimizations. Just let me know if you have any questions on this at all, I will properly communicate this when the time comes though.

For the whole dots thing, we've got it now as a feature, and I'm pretty pleased the way it's worked out here.

Note the 0.19 release with this will still be a little while (perhaps another week). Thanks for your patience!

@johnpapa
Copy link
Author

Are we close to 0.19? I'm anxious for this dot support :)

@wardbell
Copy link

Me too! Read this thread with avidity. I too use a lot of '.'s. to distinguish types of things, e.g. x.spec.js y.component.ts, y.component.spec.ts

I'm using dashes for now. Would prefer to have a small renaming party in the near future rather than a big renaming party later.

@johnpapa
Copy link
Author

I tried this out by installing from master and it works great with dots in the filenames.

@guybedford
Copy link
Member

Still working on the 0.19 here, it's almost ready to go, but there are a lot of checks to do for a major through the whole stack, including testing building and bundling changes.

@guybedford
Copy link
Member

Released in 0.19.0.

@johnpapa
Copy link
Author

thanks. testing now

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

No branches or pull requests

4 participants