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

Fix #87 #92

Merged
merged 4 commits into from
Jun 7, 2017
Merged

Fix #87 #92

merged 4 commits into from
Jun 7, 2017

Conversation

lukeed
Copy link
Member

@lukeed lukeed commented Jun 4, 2017

Created a new webpack-plugin-replace in order to move forward & close #87.

I can add & update this plugin as needed 👍

As it is, I went through the reproduction steps & it has been fixed.

rkostrzewski
rkostrzewski previously approved these changes Jun 5, 2017
include: /babel-helper$/,
patterns: [{
regex: /throw\s+(new\s+)?(Type|Reference)?Error\s*\(/g,
value: 'return;('
Copy link
Member

Choose a reason for hiding this comment

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

as per slack, we should make sure to retain the offset here via space padding:

value: s => `return;${new Array(s.length-8).join(' ')}(`

@lukeed
Copy link
Member Author

lukeed commented Jun 5, 2017

Hmm... lukeed dismissed rkostrzewski’s stale review that automatic wording seems a bit harsh!

Updated with @developit's suggestion for source-mapping (via character length).

Copy link

@jmadler jmadler left a comment

Choose a reason for hiding this comment

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

Partten the intrusion, but I wanted to let you know I've confirmed this fix with a local build!

@lukeed
Copy link
Member Author

lukeed commented Jun 7, 2017

@jmadler 😆 I giggled. Glad to hear!

@developit Your lead, good sir -- I'd feel strange merging own PR.

@gokulkrishh
Copy link

gokulkrishh commented Jun 7, 2017

@developit When can we expect fixes for firebase in preact-cli ?. Working on an existing app porting from react to preact ;) got stuck with firebase uglify issue :)

@developit developit merged commit 2445c47 into master Jun 7, 2017
@developit developit deleted the fix-87 branch June 7, 2017 09:23
@developit
Copy link
Member

@gokulkrishh today!

@gokulkrishh
Copy link

@developit Cool. Thanks 😃

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.

Unable to build with firebase as import
5 participants