Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

0.15.22 regression: external requires are mapped to the main file #648

Closed
adamburgess opened this issue Jul 15, 2016 · 4 comments
Closed

Comments

@adamburgess
Copy link
Contributor

adamburgess commented Jul 15, 2016

input file: app.js

import React from 'react'
console.log(React);

0.15.21 build to cjs with react as external: jspm build app.js --format cjs --externals react

'use strict';
Object.defineProperty(exports, '__esModule', { value: true });
function _interopDefault (ex) { return (ex && (typeof ex === 'object') && 'default' in ex) ? ex['default'] : ex; }

var React = _interopDefault(require('react'));
console.log(React);
//# sourceMappingURL=build.js.map

As you can see, it require's react, as expected.

0.15.22 build to cjs with react as external: jspm build app.js --format cjs --externals react

'use strict';
Object.defineProperty(exports, '__esModule', { value: true });
function _interopDefault (ex) { return (ex && (typeof ex === 'object') && 'default' in ex) ? ex['default'] : ex; }

var React = _interopDefault(require('react/react.js'));
console.log(React);
//# sourceMappingURL=build.js.map

Here it erroneously require's react/react.js.

The bug is still there in 0.15.23.

@adamburgess
Copy link
Contributor Author

so the fix in eabc810 wasn't perfect, it seems

@adamburgess adamburgess changed the title 0.15.22 regression: default external requires are normalised 0.15.22 regression: default external requires are mapped to the main file Jul 21, 2016
@adamburgess adamburgess changed the title 0.15.22 regression: default external requires are mapped to the main file 0.15.22 regression: external requires are mapped to the main file Jul 21, 2016
@guybedford guybedford added the bug label Jul 21, 2016
@guybedford
Copy link
Member

@adamburgess right, we should extend the function getAlias to do package detection and remove mains at https://github.com/systemjs/builder/blob/master/lib/utils.js#L77 before running the reverse map configuration.

The package lookup is provided at https://github.com/systemjs/builder/blob/master/lib/utils.js#L302 so this should be as simple as something like:

var packageName = getPackage(loader.packages, loader.decanonicalize(canonicalName));
var packageMain = packageName && loader.packages[packageName].main;
if (packageMain && canonicalName.substr(canonicalName.length - packageMain.length - 1) == '/' + packageMain)
  canonicalName = canonicalName.substr(0, canonicalName.length - packageMain.length - 1);

@guybedford
Copy link
Member

Added in 5e72725.

@guybedford
Copy link
Member

Released in 0.15.24.

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

No branches or pull requests

2 participants