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

umd and es targets instead of import source #234

Merged
merged 3 commits into from
Mar 19, 2018
Merged

umd and es targets instead of import source #234

merged 3 commits into from
Mar 19, 2018

Conversation

thiamsantos
Copy link
Contributor

That should close #233.

But needs testing to prevent #229 and #231 to happen again.

@imevro
Copy link
Collaborator

imevro commented May 18, 2017

Can you test?

@thiamsantos
Copy link
Contributor Author

Sure. Right now I can't, but I will do it when I get home.

@thiamsantos
Copy link
Contributor Author

Sorry for the delay, I will try to test it in this weekend.

@thiamsantos
Copy link
Contributor Author

I tested with rollup and webpack, and everything seems to work.

Dependencies used:

{
  "babel-core": "^6.24.1",
  "babel-loader": "^7.0.0",
  "babel-plugin-external-helpers": "^6.22.0",
  "babel-preset-env": "^1.4.0",
  "rollup": "^0.41.6",
  "rollup-plugin-babel": "^2.7.1",
  "rollup-plugin-commonjs": "^8.0.2",
  "rollup-plugin-node-resolve": "^3.0.0",
  "webpack": "^2.4.1"
}

webpack.config.js

var path = require('path')

module.exports = {
  entry: './app/main.js',
  output: {
    filename: 'bundle.js',
    path: path.resolve(__dirname, 'public')
  },
  module: {
    rules: [
      {
        test: /\.js$/,
        exclude: /node_modules/,
        use: {
          loader: 'babel-loader',
          options: {
            presets: ['env']
          }
        }
      }
    ]
  }
}

rollup.config.js

import commonjs from 'rollup-plugin-commonjs'
import nodeResolve from 'rollup-plugin-node-resolve'
import babel from 'rollup-plugin-babel'

export default {
  entry: 'app/main.js',
  format: 'iife',
  plugins: [
    babel({
      exclude: 'node_modules/**',
      presets: [
        [
          'env',
          {
            modules: false
          }
        ]
      ],
      plugins: [
        'external-helpers'
      ]
    }),
    nodeResolve({
      jsnext: true,
      module: true,
      main: true,
      browser: true
    }),
    commonjs()
  ],
  dest: 'public/bundle.js'
}

@maxrbaldwin
Copy link

maxrbaldwin commented Jul 26, 2017

Still getting the Module not found: Error: Can't resolve 'redux-logger' error. This seems to be the fix. Will this get merged any time soon? @thiamsantos @evgenyrodionov

package.json Outdated
@@ -38,8 +41,7 @@
]
},
"files": [
"dist",
"src"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you bring back src folder? I think some users can rely on this and I don't see reasons break it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@imevro imevro changed the title fix #233 umd and es targets instead of import source Aug 16, 2017
@codecov-io
Copy link

codecov-io commented Aug 16, 2017

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #234   +/-   ##
=====================================
  Coverage      82%    82%           
=====================================
  Files           5      5           
  Lines         150    150           
=====================================
  Hits          123    123           
  Misses         27     27

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 e721e17...507611b. Read the comment docs.

@thiamsantos
Copy link
Contributor Author

Any thoughts @evgenyrodionov?

@itsjoekent
Copy link

Any updates on this being merged?

@thiamsantos
Copy link
Contributor Author

Any updates on this @evgenyrodionov ?

@grushetsky
Copy link
Collaborator

grushetsky commented Mar 18, 2018

@thiamsantos, could you, please, resolve conflicts by rebasing to the latest master?

@thiamsantos
Copy link
Contributor Author

@grushetsky conflicts resolved.

@grushetsky
Copy link
Collaborator

@evgenyrodionov, let's merge this one, do you mind?

@imevro
Copy link
Collaborator

imevro commented Mar 19, 2018

@grushetsky grushetsky merged commit 3ca9f2c into LogRocket:master Mar 19, 2018
@gregorym
Copy link

Has this been released in 3.0.6? I'm still seeing this issue with version 3.0.6

@adipascu
Copy link

adipascu commented Aug 29, 2018 via email

@gregorym
Copy link

@evgenyrodionov Is something preventing a new release (3.0.7)? This has been merged months ago.

@d4rky-pl
Copy link

Bump

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.

createLogger doesn't seem to be exported correctly in latest build
9 participants