Skip to content

Conversation

@alandipert
Copy link
Collaborator

We'd previously introduced a package.json to support our JS unit tests; this PR adds a couple devDependencies and upgrades a few others. A yarn install will be required after merging since package.json and yarn.lock have changed.

JS testing is still performed the same way, by running yarn test.

In addition, the following files were added:

  • srcjs/react-tools.js: This is the input file to Babel's transformations. Running yarn run webpack compiles this file into inst/www/react-tools/react-tools.js which was previously hand-edited but should still be version-controlled. After this PR, it should not be edited by hand. I copied the source file to its new location without changing any syntax. I figure we can do that moving forward, incrementally, since we'll have support for it.
  • inst/www/react-tools/react-tools.js.map: Source map that corresponds to react-tools.js.map and improves the debugging experience. Should also be version-controlled.
  • webpack.config.js: We used to have a webpack configuration, but it was embedded in karma.conf.js and used only for building the tests. Now, it's a separate file, and contains some additional configuration. karma.conf.js does not duplicate this configuration. Instead, it imports webpack.config.js as a module and refers to its module configuration property. This means that changes to the webpack configuration can be made in one place and they should be picked up both when building release JS and when testing JS.

var path = require('path');

module.exports = {
entry: path.join(__dirname, 'srcjs', 'react-tools.js'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add mode: 'development', since react-tools.js is so small? Or do you plan to import lots of code? I prefer to have a non-minified file since some authors might not be as familiar with debugging using a sourcemap.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timelyportfolio makes sense; and I have no plans to import code. I pushed an edit to webpack.config.js and built new inst/ JS.

@timelyportfolio timelyportfolio merged commit abd64e5 into master Feb 11, 2019
@timelyportfolio
Copy link
Collaborator

Thanks @alandipert!!!

@alandipert alandipert deleted the webpack branch February 11, 2019 17:43
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 this pull request may close these issues.

3 participants