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

[BUG] Code splitting not working. #17

Open
mnlbox opened this issue Jun 27, 2020 · 11 comments
Open

[BUG] Code splitting not working. #17

mnlbox opened this issue Jun 27, 2020 · 11 comments

Comments

@mnlbox
Copy link

mnlbox commented Jun 27, 2020

I checked the code splitting branch and it seems it's not working. After doing changes and try using the component-library in a test app (created by CRA) I got this error: (I build and re-install the package after add code splitting and run yarn cache clean)

./src/App.tsx
Cannot find module: 'react-component-library/build/TestComponent/TestComponent'. Make sure this package is installed.

You can install this package by running: yarn add react-component-library/build/TestComponent/TestComponent.


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

I tried to access TestComponent exactly the same as you told in your article, like this:

import TestComponent from 'react-component-library/build/TestComponent/TestComponent';

Also, this is the folder structure after code-splitting contains folders like node_modules and src and it's not clean enough:

.
├── index.d.ts
├── node_modules
│   └── style-inject
│       └── dist
│           ├── style-inject.es.js
│           └── style-inject.es.js.map
├── src
│   ├── index.js
│   ├── index.js.map
│   ├── TestComponent
│   │   ├── index.js
│   │   ├── index.js.map
│   │   ├── TestComponent.js
│   │   ├── TestComponent.js.map
│   │   ├── TestComponent.scss.js
│   │   └── TestComponent.scss.js.map
│   └── TestComponentNew
│       ├── index.js
│       ├── index.js.map
│       ├── TestComponentNew.js
│       ├── TestComponentNew.js.map
│       ├── TestComponentNew.scss.js
│       └── TestComponentNew.scss.js.map
├── TestComponent
│   ├── index.d.ts
│   ├── TestComponent.d.ts
│   └── TestComponent.types.d.ts
├── TestComponentNew
│   ├── index.d.ts
│   ├── TestComponentNew.d.ts
│   └── TestComponentNew.types.d.ts
├── typography.scss
└── variables.scss

It's better to able import component in code splitting like this:

import TestComponent from 'react-component-library/TestComponent';

Rather than this:

import TestComponent from 'react-component-library/build/TestComponent/TestComponent';
@HarveyD
Copy link
Owner

HarveyD commented Jun 27, 2020

Issue was resolved in: #14

Please update the branch and try again.

In regards to your second point, the best we can do is:

import TestComponent from 'react-component-library/build/TestComponent';

If we use index.ts files in our components and only target one module format (CJS). We can't easily remove the build directory due as that would involve outputting the compiled code in the top level directory. If you're okay with that, change dir from build to '' and update main to 'index.js'

@mnlbox
Copy link
Author

mnlbox commented Jun 27, 2020

@HarveyD Thanks, it's worked. Just one question:
I tried to add a new component exactly the same as TestComponent called TestComponentNew and using code-splitting now. But the bundle size for my test app (a CRA application) not changed when I using just TestComponent or both components.
It seems it's put the whole component in the bundle in both cases.
Do you have any idea about this?

@HarveyD
Copy link
Owner

HarveyD commented Jun 28, 2020

You have to add an entrypoint to the new component in rollup-config.js:

input: ["src/index.ts", "src/TestComponent/index.ts"],

@mnlbox
Copy link
Author

mnlbox commented Jun 28, 2020

@HarveyD I added there. I don't know why but this config didin't work for me because when I add preserveModules: true, my build is like this:

├── index.d.ts
├── node_modules
│   └── style-inject
│       └── dist
│           ├── style-inject.es.js
│           └── style-inject.es.js.map
├── src
│   ├── index.js
│   ├── index.js.map
│   ├── TestComponent
│   │   ├── index.js
│   │   ├── index.js.map
│   │   ├── TestComponent.js
│   │   ├── TestComponent.js.map
│   │   ├── TestComponent.scss.js
│   │   └── TestComponent.scss.js.map
│   └── TestComponentNew
│       ├── index.js
│       ├── index.js.map
│       ├── TestComponentNew.js
│       ├── TestComponentNew.js.map
│       ├── TestComponentNew.scss.js
│       └── TestComponentNew.scss.js.map
├── TestComponent
│   ├── index.d.ts
│   ├── TestComponent.d.ts
│   └── TestComponent.types.d.ts
├── TestComponentNew
│   ├── index.d.ts
│   ├── TestComponentNew.d.ts
│   └── TestComponentNew.types.d.ts
├── typography.scss
└── variables.scss

As you can see I have one node_modules inside this build folder that not clean. Also for example for TestComponent component my types is located in TestComponent folder but js modules located in src/TestComponent. So we can't import module like as you told:

import TestComponent from 'react-component-library/build/TestComponent';

I think something is missing here. 🤔

@HarveyD
Copy link
Owner

HarveyD commented Jul 1, 2020

I've updated the branch. You'll need to rebase on it, or look at what I've changed (removed rollup-plugin-postcss) and make those changes :)

@mnlbox
Copy link
Author

mnlbox commented Jul 6, 2020

@HarveyD I applied all of your changes (remove @rollup/plugin-node-resolve as you did in this commit 8541820) but my output folder structure still is messy like bellow:

.
├── node_modules
├── src
├── typography.scss
├── variables.scss
└── _virtual

I change this line

"declarationDir": "build",
to "declarationDir": "build/src" and it's a little cleaner now but still I don't know why I have node_modules and _virtual folder here. 🤔
Any idea?

@mnlbox
Copy link
Author

mnlbox commented Jul 6, 2020

Also when I want to use this component library in a react app based on create-react-app I gave a lot of error like this:

import { Button } from "component-library/build/src/Button"
[at-loader] ./node_modules/component-library/node_modules/@types/lodash/ts3.1/common/function.d.ts:594:10 
    TS2300: Duplicate identifier '__'.

[at-loader] ./node_modules/component-library/node_modules/@types/lodash/ts3.1/common/function.d.ts:595:10 
    TS2300: Duplicate identifier 'Function0'.

[at-loader] ./node_modules/component-library/node_modules/@types/lodash/ts3.1/common/function.d.ts:596:10 
    TS2300: Duplicate identifier 'Function1'.



[at-loader] ./node_modules/component-library/node_modules/@types/react/index.d.ts:3115:13 
    TS2717: Subsequent property declarations must have the same type.  Property 'feTurbulence' must be of type 'SVGProps<SVGFETurbulenceElement>', but here has type 'SVGProps<SVGFETurbulenceElement>'.

[at-loader] ./node_modules/component-library/node_modules/@types/react/index.d.ts:3116:13 
    TS2717: Subsequent property declarations must have the same type.  Property 'filter' must be of type 'SVGProps<SVGFilterElement>', but here has type 'SVGProps<SVGFilterElement>'.

It sees it's about our dependencies types: lodash, react, ...
It's also intresting because we didn't use lodash and we are using lodash-es currently 🤔

@mnlbox
Copy link
Author

mnlbox commented Jul 7, 2020

@HarveyD My issue fixed after install and add this module to rollup config:
https://github.com/stevenbenisek/rollup-plugin-auto-external
Now rollup didn't create node-modules folder in my output.
Maybe you can also check it and it's necessary we can add this to the related branch. 😉

@HarveyD
Copy link
Owner

HarveyD commented Jul 11, 2020

Yeah it seems like code splitting and bundling 3rd party dependencies don't play nicely together. The plugin you mentioned checks all the dependencies and prevents them from being bundled (and the node-modules output in the build folder).

This means you'll have to include these 3rd party libraries in projects using your component library. As such, I recommend including them as peerDependencies. The peerDepsExternal plugin will also prevent them from being bundled.

@mnlbox
Copy link
Author

mnlbox commented Jul 13, 2020

@HarveyD Thanks for your answer.
Any way to prevent the creation of _tslib.js in preserveModules mode? I think this is the last issue on code splitting build folder structure 😉

@goldins
Copy link

goldins commented Oct 20, 2020

This is sort of unrelated but I saw that you replaced the postcss plugin with sass as part of the fix for this.

When using the sass plugin with @use ../ SASS directives instead of @import, there are various errors such as Error: Expected digit.

I haven't looked into the cause of this, but it's similar behavior in a project of mine.

To reproduce, update TestComponent.scss to use @use:

@use "../../variables";
@use "../../typography";

.test-component {
  background-color: variables.$harvey-white;
  border: 1px solid variables.$harvey-black;
  padding: 16px;
  width: 360px;
  text-align: center;

  .heading {
    @include typography.heading;
  }

  &.test-component-secondary {
    background-color: variables.$harvey-black;
    color: variables.$harvey-white;
  }
}

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

No branches or pull requests

3 participants