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

Cannot scan some TypeScript compiler CJS output #37

Closed
FredKSchott opened this issue Mar 1, 2021 · 4 comments · Fixed by #40
Closed

Cannot scan some TypeScript compiler CJS output #37

FredKSchott opened this issue Mar 1, 2021 · 4 comments · Fixed by #40

Comments

@FredKSchott
Copy link
Contributor

/cc @BPScott
https://unpkg.com/@shopify/[email protected]/dist-modern/index.js

'use strict';
Object.defineProperty(exports, '__esModule', {value: true});
exports.colorFactory = void 0;
var color_factory_1 = require('./color-factory');
Object.defineProperty(exports, 'colorFactory', {
  enumerable: true,
  get: function () {
    return color_factory_1.colorFactory;
  },
});
{ exports: [ '__esModule' ], reexports: [] }

cjs-module-lexer properly scans __esModule, but does not scan colorFactory. Is this expected? If so, is the presence of get instead of value also enough to indicate an export?

@BPScott
Copy link

BPScott commented Mar 1, 2021

This code was generated from using the Typescript 4.1.5 compiler outputting to cjs, by setting "module": "commonjs" option in tsconfig.json. The source for @shopify/polaris-tokens is available at https://github.com/shopify/polaris-tokens.

The source code that produces the output in dist-modern/index.js is:

export {colorFactory} from './color-factory';

Here's a TS playground demonstrating this output

@BPScott
Copy link

BPScott commented Mar 1, 2021

Looks like TS changed its output in TS 3.9.x. Contrast 3.8 output vs 3.9 output.

microsoft/TypeScript#35967 and rollup/plugins#556 seem related

@guybedford
Copy link
Collaborator

Turns out this is a duplicate of #31 and not actually related to the TypeScript output change but the trailing comma added to the property definition.

Will aim to backport the fix on Node.js as far as possible, but bear in mind Node.js 12 support isn't assured at this point.

@BPScott
Copy link

BPScott commented Mar 7, 2021

Great hunting. Thank you @guybedford!

Turns out "output from tsc" and "output from tsc, then unknowingly ran though prettier" are two subtly different things. Sorry for making that confusing :)

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

Successfully merging a pull request may close this issue.

3 participants