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

Update jugglingdb to 2.0.1 #301

Merged
merged 2 commits into from
Mar 8, 2019
Merged

Update jugglingdb to 2.0.1 #301

merged 2 commits into from
Mar 8, 2019

Conversation

styfle
Copy link
Member

@styfle styfle commented Mar 6, 2019

This fixes a potential security vulnerability.

Related to 1602/jugglingdb#460

Fixes: #303

@styfle styfle requested a review from guybedford March 6, 2019 15:09
@styfle
Copy link
Member Author

styfle commented Mar 6, 2019

✕ should execute "ncc run jugglingdb.js" (818ms)

@guybedford I broke it 😞

I'm not sure what the original use case was here.

Should we continue testing the old version and add a new test for the new version? 🤔

@rauchg
Copy link
Member

rauchg commented Mar 6, 2019

Tests are failing

@guybedford
Copy link
Contributor

I've reported on this case here - #303.

@guybedford
Copy link
Contributor

I've just pushed an update to the asset relocator loader here, which goes with the approach (1) described in #303. This does have some minor perf cost (roughly 5%), but allows this case to be built and handled properly, ensuring we only build one copy of jugglingdb.

@styfle styfle requested a review from rauchg March 8, 2019 15:09
@rauchg rauchg merged commit d5d4a92 into master Mar 8, 2019
@rauchg rauchg deleted the update-jugglingdb branch March 8, 2019 17:49
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.

Handling conditional require errors
3 participants