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

[refactor] we cannot import files directly anymore if we want to embrace pure es6 import syntax #383

Closed
tyn1998 opened this issue Jun 24, 2022 · 3 comments

Comments

@tyn1998
Copy link
Member

tyn1998 commented Jun 24, 2022

Description

A few months ago

"keep_fnames: ture" option is for a really-hard-to-detect problem:
image

As we know, webpack will try its best to minimize the bundles size in build mode and one of its weapons is to substitute long variable names with short ones. Each module share a common name transformation rule because each module has its own name space when imported properly.

The problem in our code is, we import files directly by import './xxx', this makes 6 transformed t to be exposed in same name space and then they conflict.

So keep founction names when building bundles.

Originally posted by @tyn1998 in #274 (comment)

Today

I'm working on #381 today, and I find:

  • build produced by yarn run start works well
  • build produced by yarn run build fails to make contentscripts work, but Popup and Options work well

Finally I recall the 't' problem which I found a few months before and disable all import './xxx' but import ./PerceptorTab:

image

Then PerceptorTab shows up:

image

Solution

This problem is related to:

So I think the refactoring work should be paused for a while until #347 comes up.

@menbotics menbotics bot added kind/bug Category issues or prs related to bug. kind/enhancement Category issues or prs related to enhancement. labels Jun 24, 2022
@tyn1998 tyn1998 assigned tyn1998 and unassigned tyn1998 Jun 24, 2022
@tyn1998 tyn1998 removed kind/enhancement Category issues or prs related to enhancement. kind/bug Category issues or prs related to bug. labels Jun 24, 2022
@tyn1998
Copy link
Member Author

tyn1998 commented Jul 7, 2022

Forgot to remove this:

image

This issue will be closed after this webpack option is removed.

@tyn1998
Copy link
Member Author

tyn1998 commented Jul 7, 2022

Well, I found that no matter importing classes by files or modules their constructor.name will always be 't' if having keep_fnames not being true in production mode. (also refer to this)

So just keep the option enabled if we still use the current inject2Perceptor mechanism.

@tyn1998 tyn1998 closed this as completed Jul 7, 2022
@tyn1998
Copy link
Member Author

tyn1998 commented Jul 7, 2022

  • build produced by yarn run build fails to make contentscripts work, but Popup and Options work well

破案了!

The terserOptions.keep_fnames seemed not work and actually it didn't work because process.env.BABEL_ENV was not set to 'production' as we expected.

image

The problem was introduced by the transformation from commonjs modules to es6 modules. The code above worked well in require syntax but not in import syntax.

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

1 participant