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

Support typescript:main, module, and jsnext:main in package.json #109

Closed
appsforartists opened this issue Jan 16, 2017 · 9 comments
Closed
Assignees

Comments

@appsforartists
Copy link

In package.json, main refers to a file that uses CommonJS to handle importing/exporting other files. There are other standards for other module formats:

For TypeScript, typescript:main
For ES2015 import/export statements, module and jsnext:main

The relevant Pundle presets (e.g. TypeScript and Babel) should prefer these keys by default.

@steelbrain steelbrain self-assigned this Jan 16, 2017
@steelbrain
Copy link
Owner

Sounds fair

@appsforartists
Copy link
Author

This is especially nice if you're working across many modules. It lets you edit the source of a dependency and still get HMR without needing to run a build step in the middle.

@steelbrain
Copy link
Owner

Fixed as of 0d7214b
I've added module, but not jsnext:main mainly because it's specific to a package and we want to keep things like that configurable to the user. but module seems more like a standard and also supersedes jsnext:main so that shouldn't be a problem.

@appsforartists
Copy link
Author

module seems to work with TypeScript. I tested by importing this module:

appsforartists/MidiConvert@6bdb7de

Want to add it back to the TS preset for beta9?

@steelbrain
Copy link
Owner

Readded as of 27420f8

@appsforartists
Copy link
Author

Using this config in .pundle:

  presets: [
    [
      require.resolve('pundle-preset-default'),
      {
        loader: false,
      }
    ],
    [
      require.resolve('pundle-preset-typescript'),
      {
        resolver: {
          packageMains: ['typescript:main', 'browser', 'main'],
        },
        transformer: {
          extensions: ['js', 'jsx', 'ts', 'tsx'],
          exclude: [],
          config: {
            compilerOptions: Object.assign(
              mainTSConfig.compilerOptions,
              tsConfig.compilerOptions
            )
          }
        }
      }
    ],
  ],

Resolver is seeing this config:

{ alias: {},
  extensions: null,
  packageMains: [ 'browser', 'browserify', 'webpack', 'main' ],
  modulesDirectories: [ 'node_modules' ],
  knownExtensions: [ '', 'js', 'jsx', 'json', 'ts', 'tsx' ] }

In all places that config.packageMains appears in this file:

https://github.com/motion/pundle/blob/f893148bd7e322a36754d9ddc00f5bf80d70bea0/packages/resolver-default/src/index.js#L80

Moving the resolver block into pundle-preset-default corrects packageMains. Are you sure the resolver is looking in other presets for its configuration.

@steelbrain
Copy link
Owner

In addition to having loader: false, you should also do resolver: false in default preset. The problem is ocurring because default loader is resolving it before it gets to the typescript loader, and exts are the same between the two

@appsforartists
Copy link
Author

Adding resolver: false breaks imports:

  Error   Cannot find module './lib/ReactDOM' from 'node_modules/react-dom/index.js'

@steelbrain steelbrain reopened this Jan 21, 2017
@steelbrain
Copy link
Owner

I've put a working pundle configuration for your project in a gist. It doesn't look very nice right now, and is not the most pleasant thing to configure.

I'm going to change that in the upcoming releases so the end user only has to specify extensions or any other config once per preset and the preset can apply that one config at multiple places.

I've opened #117 to track it

appsforartists added a commit to material-motion/material-motion-js that referenced this issue Jan 23, 2017
Summary: Sharing `extensions` across config dictionaries as recommended by @steelbrain in steelbrain/pundle#109

Reviewers: O2 Material Motion, O3 Material JavaScript platform reviewers, #material_motion, steelbrain

Reviewed By: steelbrain

Subscribers: steelbrain

Tags: #material_motion

Differential Revision: http://codereview.cc/D2531
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

2 participants