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

Catch unexpected chars at build time #5424

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jan 19, 2021

Guard against issue #5382.

@plotly/plotly_js

@archmoj archmoj added this to the NEXT milestone Jan 19, 2021
@archmoj
Copy link
Contributor Author

archmoj commented Jan 19, 2021

Also noticed in #5416 (comment).

@alexcjohnson
Copy link
Collaborator

I'm not quite sure I understand this rule, it looks like it only applies to regular expressions in the code. Can you verify that this command fails prior to the fix in #5382?

@archmoj
Copy link
Contributor Author

archmoj commented Jan 20, 2021

I'm not quite sure I understand this rule, it looks like it only applies to regular expressions in the code. Can you verify that this command fails prior to the fix in #5382?

Yes I did verify that. Eslint catched the problem. But I am not 100% sure about that rule. Maybe we could just run eslint with an empty rule?

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Maybe we could just run eslint with an empty rule?

Haha interesting concept - yes, I suppose if eslint is simply unable to parse the file it would fail even without a rule. Anyway since you've shown this is a valuable check, let's go with it as is. 💃

@archmoj archmoj merged commit 5f51450 into master Jan 20, 2021
@archmoj archmoj deleted the guard-unexpected-chars-in-bundles branch January 20, 2021 00:16
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.

2 participants