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

Add module and types settings in package.json for wouter and wouter/preact #55

Merged
merged 6 commits into from
Jul 18, 2019
Merged

Conversation

cedeber
Copy link
Contributor

@cedeber cedeber commented Jul 12, 2019

These settings are useful for tools like pikapkg which require the module settings in order to build wouter. Adding a private package.json into the /preact folder allow to do import [..] from "wouter/preact";

I don't add the main setting because most bundlers are waiting for ES5 here, which wouter doesn't provide. And that's ok ;)

@cedeber
Copy link
Contributor Author

cedeber commented Jul 12, 2019

The build failure happens because of the /preact/package.json file. No idea how to solve this problem.

@Ty3uK
Copy link
Contributor

Ty3uK commented Jul 13, 2019

@cedeber To solve that issue move Babel config from package.json to babel.config.js file.

Also latest preact version is 10.0.0-rc.0

@codecov-io
Copy link

codecov-io commented Jul 13, 2019

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #55   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines         102    102           
  Branches       26     26           
=====================================
  Hits          102    102

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6cbd0b...d2c3c1b. Read the comment docs.

@cedeber
Copy link
Contributor Author

cedeber commented Jul 13, 2019

@Ty3uK Thanks for the help. Looks good now.

@molefrog
Copy link
Owner

@cedeber @Ty3uK Thanks for the help guys. I'm gonna try to review this one Monday.

babel.config.js Outdated
@@ -0,0 +1,11 @@
module.exports = function(api) {
api.cache(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Missed indentation?

babel.config.js Outdated
api.cache(true);

const presets = ["@babel/preset-env"];
const plugins = ["@babel/plugin-transform-react-jsx"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just write

module.exports = {
  presets: ["@babel/preset-env"],
  plugins: ["@babel/plugin-transform-react-jsx"]
}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks new for Babel 7. Apparently setting the cache to true keeps the config in memory for each calls instead of reading it each time. See https://babeljs.io/docs/en/config-files#apicache. Both work. I don't care which one is preferred ;) I simply did it the way it is documented.

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad, I mean

return {
  presets: ["@babel/preset-env"],
  plugins: ["@babel/plugin-transform-react-jsx"]
}

without unnecessary variables

Copy link
Owner

@molefrog molefrog left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This all looks good, I am going to merge it and release a new version this week.

@molefrog molefrog merged commit 408dff7 into molefrog:master Jul 18, 2019
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.

None yet

4 participants