csstools / postcss-normalize

Use the parts of normalize.css (or sanitize.css) you need from your browserslist
Creative Commons Zero v1.0 Universal
816 stars 40 forks source link

UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'indexOf' of undefined #16

Closed yhuard closed 7 years ago

yhuard commented 7 years ago

First of all, thanks for your work @jonathantneal

I'd like to report a bug I'm facing while trying to integrate your plugin. Here's my script: https://github.com/ec-europa/ecl-toolkit/blob/8f38812e4675dc7c7df63715109671a7137a2ee9/packages/ecl-builder/scripts/styles.js#L20

If I remove the lines 20-22 (those adding the postcss-normalize plugin), all works fine. Any idea?

Error message:

UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'indexOf' of undefined

I use Node.js v6.9.5

Thanks!

jonathantneal commented 7 years ago

Thanks for filing this and for showing me exactly where things are going wrong.

When you run this, does process.env.NODE_ENV === 'production'?

I ask because I want to test this plugin coming before cssnano or autoprefixer to see if that is causing the issue. If I have time, I’ll just test both ASAP.

Thanks again!

jonathantneal commented 7 years ago

Could you give me some guidance on spinning up your project? I’ll dive in.

So far, I have 2 additional tests for PostCSS Normalize:

Both are passing. Have you run npm update from your project recently? I’m wondering if you’re locally running an older version of PostCSS Normalize?

yhuard commented 7 years ago

Whether I run this plugin before or after, with or without other plugins like cssnano, it doesn't change anything. postcss([postcssNormalize()]) produces the same result. I'll test your 2 additional tests and let you know

jonathantneal commented 7 years ago

I’m happy to test, too.

I cloned ecl-toolkit, ran npm install, went into examples/basic, ran npm start, and the page works with no errors in my console, but I also don’t think it’s running my plugin.

How might I test this to see the break you have?

yhuard commented 7 years ago

This is interesting. I tried with npm instead of yarn as you did with a clean install (rm -rf node_modules && npm i) and it works (in ecl-toolkit).

yhuard commented 7 years ago

The only difference I see: usually, I only run yarn at the root of the project. Internally, it runs lerna bootstrap. It runs yarn inside examples/basic. Since ecl-builder is also managed by lerna, it doesn't get actually installed but symlinked. Which means that postcss-normalize isn't copied into examples/basic/node_modules but stays in packages/ecl-builder/node_modules.

So, could it be a path solving issue?

jonathantneal commented 7 years ago

I’m a bit green to yarn.

I ran yarn from the root of the project. Then I went into examples/basic and ran yarn again. Then I ran yarn start and the server started, and I didn’t see any errors in my console, but again I’m not sure PostCSS Normalize is running.

Would you have advice on how I might recreate this? I’m definitely willing to try whatever and get this resolved for you ASAP.

yhuard commented 7 years ago

You should not run yarn in examples/basic, lerna already (kind of) did it when you ran yarn at the root level. If you run yarn in examples/basic, it will actually fetch the latest version published on npm, which is not the local version I want to test.

The exact steps are:

  1. clone the repo
  2. run yarn
  3. cd examples/basic then yarn start
jonathantneal commented 7 years ago

I just realized one important step that was missing:

git checkout feat/postcss-normalize

Trying again.

jonathantneal commented 7 years ago

Okay. I was able to toss in some console.log statements around Line 47 in the node_modules copy of the plugin, and I verified that the error is not coming from my plugin.

That doesn’t mean we’re out of the woods, yet. So, I’m still looking.

yhuard commented 7 years ago

Commenting the line root.prepend(normalizeRules); makes the error disappear. Progressing...

jonathantneal commented 7 years ago

This might be a bug with PostCSS.

I was able to resolve the error by adding this on a new line after Line 12 of json-to-ast.js:

{ source: {} }

Diving in to see if there’s another way to handle this.

yhuard commented 7 years ago

Adding json.type !== 'root' ? { source: {} } : {} instead of { source: {} } also works. When json.type === 'root', json.source.input is not empty. In the other cases, json.source.input is empty. Don't know if it helps

jonathantneal commented 7 years ago

I’ll cut a new release today with a fix, but I won’t close this until you verify it’s working on your end. I’m very sorry about this bug and I hope you’ll still consider using the project. :)

yhuard commented 7 years ago

Thanks for your help @jonathantneal! I'm waiting for the package to be updated on npm (still 2.0.1 at the time of writing). Will probably test tomorrow.

By the way, how would you handle normalize.css updates? Right now, you're using v6.0.0 even though v.7.0.0 is out for a few weeks - between us, I prefer the v6.0.0 as it removes the opinionated rules. Do you think it would make sense to have a normalize-browser-list.json manifest directly in https://github.com/necolas/normalize.css and then let the user decide which (compatible) version of normalize.css he'd like to use with this plugin?

jonathantneal commented 7 years ago

@yhuard, updated and published!

yhuard commented 7 years ago

Thank you @jonathantneal, it works fine! This issue can be closed but out of curiosity, what about the best way to handle normalize.css updates (comment above https://github.com/jonathantneal/postcss-normalize/issues/16#issuecomment-304385996)?