You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I agree with @IgorMinar - I don't see any reason for this config after migration to a newer SystemJS. More generally, I don't think we should be in business in providing System config. Going to send Pr to remove this.
@jpsfs while it is true that never systemjs moves to URLs as module modifiers it doesn't matter much in a bundled version since everything is pre-loaded and no HTTP / Ajax request should be done at runtime for ng2-specific resources.
We could register things with .js extension but this would be non-trivial (?) with out current setup in the repo where imports are shared between *.ts and *.dart files.
I added that line, as the original goal of the bundle was to make getting off the ground as quick and painless as possible. That allowed for demoes to have zero system.config, and jump straight to System.import.
So why not replace it with the 0.18 version defaultJsExtension: true, so that demo users don't feel like they need to learn system.js configuration options. Or my assumption that most demos need defaultJsExtension: true is wrong?
@rkirov I see your point about trying to make it easier on users. The pb I can see with this approach is that we are enforcing config that impacts all dependencies / everything that goes through SystemJS. So I'm afraid that it might mess up things for more experienced SystemJs users.
I'm not against adding defaultJsExtension: true but I would prefer that we, as Angular, don't try to enforce config for an app-global / wide loader.
@pkozlowski-opensource I'm not an expert on SystemJS, please correct me if I'm wrong, but if the modules are not registered with ".js", then using defaultJsExtension: true, SystemJS will try to look for the modules with the extension and will not find them.
Actually,I think this is not happening right now because of
When you remove it, the problem I just describe will appear.
Example:
import * from "angular2/src/di/metadata";
will trigger a search for "angular2/src/di/metadata.js" (because of the defaultJsExtension: true flag) and that will not be registered. (if you don't use the "":".js" config)
My vote would be to generate the bundle with the .js extension on the register. Something like:
@jpsfs I don't think it works like this. SystemJS will first consult its internal registry to see if a dependency is there. Only if a dependency is not registered SystemJS will try to fetch it and this is where defaultJsExtension comes into play.
Did you run into actual problems somewhere? If so, could you share a plunk demonstrating those?
@pkozlowski-opensource yes, you are right. But the fact is that a dependency named "angular2/src/di/metadata.js"is not registered, only "angular2/src/di/metadata" is.
Activity
fix(bundle): don't include System.config in dev bundle
pkozlowski-opensource commentedon Aug 27, 2015
I agree with @IgorMinar - I don't see any reason for this config after migration to a newer SystemJS. More generally, I don't think we should be in business in providing System config. Going to send Pr to remove this.
jpsfs commentedon Aug 27, 2015
Just a quick question, using the new version of SystemJS shouldn't all modules be registered with the JS extension?
Example:
Before (system ~0.16)
After (system ~0.18)
@pkozlowski-opensource
pkozlowski-opensource commentedon Aug 27, 2015
@jpsfs while it is true that never systemjs moves to URLs as module modifiers it doesn't matter much in a bundled version since everything is pre-loaded and no HTTP / Ajax request should be done at runtime for ng2-specific resources.
We could register things with .js extension but this would be non-trivial (?) with out current setup in the repo where imports are shared between *.ts and *.dart files.
rkirov commentedon Aug 27, 2015
I added that line, as the original goal of the bundle was to make getting off the ground as quick and painless as possible. That allowed for demoes to have zero
system.config, and jump straight toSystem.import.So why not replace it with the 0.18 version
defaultJsExtension: true, so that demo users don't feel like they need to learn system.js configuration options. Or my assumption that most demos needdefaultJsExtension: trueis wrong?pkozlowski-opensource commentedon Aug 27, 2015
@rkirov I see your point about trying to make it easier on users. The pb I can see with this approach is that we are enforcing config that impacts all dependencies / everything that goes through SystemJS. So I'm afraid that it might mess up things for more experienced SystemJs users.
I'm not against adding
defaultJsExtension: truebut I would prefer that we, as Angular, don't try to enforce config for an app-global / wide loader.IgorMinar commentedon Aug 27, 2015
the thing is that the bundle should be usable even without systemjs and in that case the config doesn't do anything.
I approved the PR to remove this. If this causes more friction elsewhere we need to find a different solution.
jpsfs commentedon Aug 27, 2015
@pkozlowski-opensource I'm not an expert on SystemJS, please correct me if I'm wrong, but if the modules are not registered with ".js", then using defaultJsExtension: true, SystemJS will try to look for the modules with the extension and will not find them.
Actually,I think this is not happening right now because of
When you remove it, the problem I just describe will appear.
Example:
will trigger a search for "angular2/src/di/metadata.js" (because of the defaultJsExtension: true flag) and that will not be registered. (if you don't use the "":".js" config)
My vote would be to generate the bundle with the .js extension on the register. Something like:
pkozlowski-opensource commentedon Aug 27, 2015
@jpsfs I don't think it works like this. SystemJS will first consult its internal registry to see if a dependency is there. Only if a dependency is not registered SystemJS will try to fetch it and this is where
defaultJsExtensioncomes into play.Did you run into actual problems somewhere? If so, could you share a plunk demonstrating those?
jpsfs commentedon Aug 27, 2015
@pkozlowski-opensource yes, you are right. But the fact is that a dependency named "angular2/src/di/metadata.js" is not registered, only "angular2/src/di/metadata" is.
I will try to reproduce this is in a plunk asap.
rkirov commentedon Aug 27, 2015
@IgorMinar sounds good, lets revisit if we end up having to remind people to set
defaultJsExtensions: true.IgorMinar commentedon Aug 27, 2015
I think that using defaultJsExtensions is now discouraged. I'm looking into
this stuff this week.
On Thu, Aug 27, 2015 at 10:47 AM Rado Kirov notifications@github.com
wrote:
angular-automatic-lock-bot commentedon Sep 7, 2019
This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.
Read more about our automatic conversation locking policy.
This action has been performed automatically by a bot.