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

Avoid function constructor in d3 #5359

Merged
merged 1 commit into from
Jan 4, 2021
Merged

Avoid function constructor in d3 #5359

merged 1 commit into from
Jan 4, 2021

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Dec 22, 2020

As mentioned in #897 (comment) [email protected] (as well as d3-dsv use function constructors in the source.

This PR suggests refactoring d3 to avoid using function constructors.

Demo

@plotly/plotly_js

package.json Outdated Show resolved Hide resolved
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.

💃 Nice - as mentioned on slack I'm comfortable with us using a fork of d3v3 in the interim until we manage to upgrade to d3v5.

@archmoj archmoj force-pushed the d3-v3-no-eval-func branch from 9f348eb to d2f7269 Compare January 4, 2021 14:20
@archmoj archmoj requested a review from alexcjohnson January 4, 2021 14:21
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.

💃

@archmoj archmoj merged commit c72264c into master Jan 4, 2021
@archmoj archmoj deleted the d3-v3-no-eval-func branch January 4, 2021 15:04
@nicolaskruchten
Copy link
Contributor

This makes sense to me, and I still think we should stop exporting these functions and do a v2.

@nicolaskruchten
Copy link
Contributor

Upon further reflection, although what I said above holds, there's no particular reason to do the v2 split immediately. I had initially thought that we would just not include the parts of d3v3 that use the function constructor, but if we're still exporting a fully-functional (although not up to Mike Bostock's performance standards!) version of d3v3 then we can just get on with things in a minor as usual.

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.

3 participants