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

Importing internal dependencies #786

Open
thiagoxvo opened this issue May 11, 2018 · 5 comments · May be fixed by #974
Open

Importing internal dependencies #786

thiagoxvo opened this issue May 11, 2018 · 5 comments · May be fixed by #974
Labels
feature refactor Code that needs to be updated

Comments

@thiagoxvo
Copy link
Contributor

I am creating a new drawing mode. To avoid reinventing the wheel I would like to reuse some functions that were already implemented in:


@mapbox/mapbox-gl-draw/src/lib/common_selectors
@mapbox/mapbox-gl-draw/src/lib/double_click_zoom

I am using create-react-app and if I try to import any of them and build my project, I have the minification issue:

Failed to minify the code from this file:
    ./node_modules/@mapbox/mapbox-gl-draw/src/lib/double_click_zoom.js:2

I am using:
mapbox-gl-js 0.44.2:
mapbox-gl-draw 1.0.7:

I could send a PR if I get a better understanding how to solve this.

@mcwhittemore
Copy link
Contributor

@thiagoxvo - To do this well, I think we need to expose these via the mode_interface. Currently all of the logic filled mode_interface functions are added to the interface via mode_interface_accessors.

I'd love to see a PR that moves common_selectors onto the mode_interface via a mode_interface_selectors file or something.

This same practice can be used for double_click_zoom.

@thiagoxvo
Copy link
Contributor Author

I didn't know about this mode_interface 🤔, should I be using this to create my modes?
I try to follow the other custom modes also the built-in modes and none of them use this mode_interface as a base mode.

@thiagoxvo
Copy link
Contributor Author

thiagoxvo commented May 11, 2018

What do you mean by exposing would be something like this?

const CommonSelectors = require('../lib/common_selectors');
ModeInterface.prototype.isVertex = function(e) {
  return CommonSelectors.isVertex(e)
};

// and later on I would be using this way ??
import MapboxDraw from '@mapbox/mapbox-gl-draw/dist/mapbox-gl-draw'

const MyAwesomeMode = MapboxDraw.modes.mode_interface
// and than I can call...
MyAwesomeMode.isVertex()

@kkaefer kkaefer added refactor Code that needs to be updated feature labels Jan 7, 2020
trygveaa added a commit to trygveaa/mapbox-gl-draw that referenced this issue Mar 20, 2020
trygveaa added a commit to trygveaa/mapbox-gl-draw that referenced this issue Mar 20, 2020
These are all the library methods that all the modes use. Exporting them
makes it possible for others to write custom modes which provides the
same functionality.

Fixes mapbox#786
@trygveaa trygveaa linked a pull request Mar 20, 2020 that will close this issue
@trygveaa
Copy link

I don't think there is any reason to expose them via the mode_interface. The library functions don't need to access this of the mode (or they take it in as a parameter if they need it), so they can just be exported normally. I created #974 which does this. Then you can just do this in your custom mode:

import MapboxGLDraw, { CommonSelectors, doubleClickZoom } from '@mapbox/mapbox-gl-draw/dist/mapbox-gl-draw'

trygveaa added a commit to trygveaa/mapbox-gl-draw that referenced this issue Mar 20, 2020
These are all the library methods that all the modes use. Exporting them
makes it possible for others to write custom modes which provides the
same functionality.

Fixes mapbox#786
@PendaGTP
Copy link

Any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature refactor Code that needs to be updated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants