Skip to content

Conversation

@bartbroere
Copy link
Contributor

@bartbroere bartbroere commented Nov 24, 2021

Although some issues still remain while performing npm run build, adding these dependencies fixes the problem with the missing modules.

#30

rules: [
{
test: /\.(js|jsx|tsx)$/,
exclude: /(node_modules)/,
Copy link
Contributor Author

@bartbroere bartbroere Nov 24, 2021

Choose a reason for hiding this comment

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

This exclusion has been removed, because we need to convert the ?? operator that appears in react-leaflet. If we can find a way to exclude all node modules again, except react-leaflet, this would probably be preferable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi Bart, Thanks for the PR!
I've been going over this issue with react-leaflet today and think I found a clean solution:
https://stackoverflow.com/a/67689949

Just using a slightly older version avoids the ?? altogether and would save us from including node_modules in webpack. I'd really like to avoid this as it increases the bundle size + triples the build time (at least on my side). I have to admit I'm far from an expert on anything babel/webpack, but this seems to be the best solution to me.

Could you try if downgrading the react-leaflet library works on your side? This is what I did:

  1. Update package.json to use "@react-leaflet/core": "1.0.2", "react-leaflet": "3.1.0"
  2. Delete the node_modules folder.
  3. (optionally, just to be sure) npm cache clean --force
  4. npm install
  5. npm run dev

If this works for you, could you update the PR to set the fixed package versions & add back the exclusion rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your reply. I also noticed excluding node_modules was not ideal for bundle size and build time. I considered making a more complex regex yesterday, but this is definitely more pragmatic.

I'll add some commits to this PR.

@nielsdejong nielsdejong merged commit 18bdeb3 into neo4j-labs:develop Nov 25, 2021
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.

2 participants