brambow / elements

Themeable React components for map-based web applications
https://cartolab-gis.github.io/elements/
MIT License
69 stars 11 forks source link

Can't resolve '../../../config' #4

Closed kylebarron closed 4 years ago

kylebarron commented 4 years ago

Describe the bug

All of yarn, yarn dev, yarn add @cartolab/elements fail.

To Reproduce

git clone https://github.com/cartolab-gis/elements
cd elements
yarn
// or yarn dev

Expected behavior

Project builds

Desktop (please complete the following information):

Smartphone (please complete the following information):

Additional context

Traceback:

> yarn add @cartolab/elements
yarn add v1.19.2
[1/4] πŸ”  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] πŸ”—  Linking dependencies...
warning " > babel-loader@8.0.6" has unmet peer dependency "webpack@>=2".
warning " > storybook-addon-emotion-theme@2.1.1" has unmet peer dependency "@storybook/addons@4.0.12".
[4/4] πŸ”¨  Building fresh packages...

success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ @cartolab/elements@1.0.1
info All dependencies
└─ @cartolab/elements@1.0.1
$ yarn build && yarn build-storybook
yarn run v1.19.2
$ yarn build-prep && yarn build-clean
$ del-cli dist && NODE_ENV=production rollup -c --context

./src/index.js β†’ dist/index.js, dist/index.esm.js...
(!) Unresolved dependencies
https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency
theme-ui (imported by src/components/_common/ElementsProvider.js)
react-icons/md (imported by src/components/Draw/Draw.js, src/components/Measure/Measure.js, src/components/Select/Select.js)
@mapbox/mapbox-gl-draw/dist/mapbox-gl-draw (imported by src/components/Measure/Measure.js, src/components/Select/Select.js)
[!] Error: Could not resolve '../../../config' from src/components/Search/util/handleSearchSubmit.js
Error: Could not resolve '../../../config' from src/components/Search/util/handleSearchSubmit.js
    at error (/Users/kyle/github/mapping/cartolab-gis-elements/node_modules/rollup/dist/rollup.js:5362:30)
    at ModuleLoader.handleMissingImports (/Users/kyle/github/mapping/cartolab-gis-elements/node_modules/rollup/dist/rollup.js:12200:17)
    at ModuleLoader.<anonymous> (/Users/kyle/github/mapping/cartolab-gis-elements/node_modules/rollup/dist/rollup.js:12091:30)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/kyle/github/mapping/cartolab-gis-elements/node_modules/rollup/dist/rollup.js:41:28)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.
brambow commented 4 years ago

Thanks for the issue. If you're cloning the source from Github you need to create and fill out a config.js file that's in the src directory. This contains your Mapbox token. There is a config.template.js file that has the structure, but you need to make your own config.js. And I need to document it!

image

{*/src/config.js*}
export default {
  mapboxToken: 'your token'
};

This shouldn't happen if you pulled the published versions from NPM because it's the dev Storybook code that's triggering this. Open to ideas for better mechanics here, or do you think better documentation is all that's needed?

brambow commented 4 years ago

Followup: I'm reading over how some other libraries approach mapbox tokens. For example: https://uber.github.io/react-map-gl/#/Documentation/getting-started/about-mapbox-tokens

I thought about using dotenv, but opted for a config file that isn't tracked in git instead.

brambow commented 4 years ago

Added documentation in #5. Closing. Will revisit if people have better ideas on how to handle this.

kylebarron commented 4 years ago

It looks like the install from npm is working now.

kylebarron commented 4 years ago

Also, I've seen one or two repositories that have like a default config.js with an empty token, so that the errors when building the project aren't as obscure, and the project builds. But then there's a warning in the terminal if the token isn't valid.

brambow commented 4 years ago

From my perspective, including an empty config.js file would increase the likelihood of accidentally exposing a Mapbox token.

We need a token during development when running Storybook, so during dev config.js needs a real token. Right now, we exclude config.js from version control so a token won't accidentally make it to Github. If it was in version control, developers would need to remove their token before any commits, or absolutely make sure not to commit changes to config.js. I think the added step of having them modify the config.template.js to config.js is worth the additional security over trying to catch accidental commits.

A better warning would be good though. I will think on ways to improve that.

kylebarron commented 4 years ago

Yeah, good points. I go the dotenv route in my projects, and just raise an error if the MapboxToken env variable isn't set.