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 additional information for postcss errors (#6282) #6352

Merged
merged 4 commits into from
Oct 23, 2019

Conversation

buildbreakdo
Copy link
Contributor

@buildbreakdo buildbreakdo commented Feb 6, 2019

Fixes #6282: If build compiler error has postcssNode property, add additional information to the error message. Default compiler CSS error messages are opaque. To verify:

npx create-react-app css-test
cd css-test
echo "label[for=*] { margin-right: 12px; }" > ./src/App.css
yarn build

Outputs error:
screen shot 2019-02-06 at 2 51 13 pm

Replace css-test/node_modules/react-scripts/build.js lines:

messages = formatWebpackMessages({
  errors: [err.message],
  warnings: [],
});

With :

        let errMessage = err.message;

        // Add additional information for postcss errors
        if (err.hasOwnProperty('postcssNode')) {
          errMessage += '\n' + path.basename(
            err['postcssNode'].source.input.file
          ) + '\nCompileError: Begins at selector ' +
            err['postcssNode'].selector +' (' +
            err['postcssNode'].source.start.line + ':' +
            err['postcssNode'].source.start.column +
            ')';
        }

        messages = formatWebpackMessages({
          errors: [errMessage],
          warnings: [],
        });

Run yarn build to view new compiler error output:
screen shot 2019-02-06 at 2 52 06 pm

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Copy link
Contributor

@mrmckeb mrmckeb left a comment

Choose a reason for hiding this comment

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

I'll just get a second opinion here. Looks good though, thanks!

@iansu
Copy link
Contributor

iansu commented Feb 25, 2019

Is the chunk name useful here? Can we point to the original source instead?

@buildbreakdo
Copy link
Contributor Author

@iansu Useful to know the file type, original source would have definitely been preferred. Understanding is that postcss is processing the chunk not generating the chunk itself, so this would be after the chunk was generated. err['postcssNode'].source.input.file is set to the chunk name.

  postcssNode:
   Declaration {
     raws: { before: '\n  ', between: ': ' },
     type: 'decl',
     parent:
      Rule {
        raws: [Object],
        type: 'rule',
        nodes: [Array],
        parent: [Object],
        source: [Object],
        selector: 'body',
        lastEach: 2,
        indexes: [Object] },
     source: { start: [Object], input: [Object], end: [Object] },
     prop: 'background',
     value: 'var(--site-background)' } }

@iansu
Copy link
Contributor

iansu commented Feb 25, 2019

I thought that might be the case. I'm just not sure how useful the chunk name is, especially in dev where the chunk isn't written to disk. I guess it can be inspected via the browser devtools?

@buildbreakdo
Copy link
Contributor Author

buildbreakdo commented Mar 5, 2019

@iansu Chunk name has been removed

EDIT:
Went ahead and removed the line number too, since this references a line in the chunk file that is not written to disk.

@stale
Copy link

stale bot commented Apr 23, 2019

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Apr 23, 2019
@iansu iansu self-assigned this Apr 24, 2019
@stale stale bot removed the stale label Apr 24, 2019
@bugzpodder bugzpodder added this to the 3.2 milestone Aug 11, 2019
@bugzpodder bugzpodder added this to In progress in v3.3 via automation Aug 11, 2019
@iansu iansu merged commit a351750 into facebook:master Oct 23, 2019
v3.3 automation moved this from In progress to Done Oct 23, 2019
@iansu
Copy link
Contributor

iansu commented Oct 23, 2019

Thanks!

@lock lock bot locked and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
v3.3
  
Done
Development

Successfully merging this pull request may close these issues.

Opaque error for invalid CSS after upgrade: TypeError: Cannot read property '0' of undefined
5 participants