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

fix: bundle entry points #8884

Merged
merged 2 commits into from
Sep 23, 2020
Merged

Conversation

farfromrefug
Copy link
Collaborator

As requested here is the PR again which fixes

This PR fixes a big issue which appeared with N 7 / esm / webpack

With webpack import / require order is not respected : webpack/webpack#1343

Now in bundle-config-loader we rely on require to load bundle-entry-points thus calling initGlobals and making tslib functions global.
Because it uses require it actually end up at the end of your main.js (simply look at a generated bundle.js)

To keep that call up we need to use import
But that is not enough!
in bundle-entry-points we use 'require' on 'globals'. And using import is not enough either because we call a method on it which will end up after all imports!

The solution to all this is call initGlobal inside global/index.ts then use import './global' for example.
Doing that i needed cleanup a bit and we end up with only on use of initGlobal

EDIT: that app will crash on ios because of that. The reason is that i import View at the top of my main.ts which include files using __metadata

# Conflicts:
#	packages/core/globals/index.ts
@cla-bot cla-bot bot added the cla: yes label Sep 23, 2020
@NathanWalker
Copy link
Contributor

I think we did some tests with import vs. require and were seeing exactly what is commented on that thread here:
webpack/webpack#1343 (comment)

Where using require as opposed to import seemed to be guarantee.

@farfromrefug
Copy link
Collaborator Author

Imports will all get at the top you are right. Which is why I changed the way initGlobals is called to ensure the right order.
In bundle config loader using require makes it end up last. Look at the generated code for a current app to see it.

@farfromrefug
Copy link
Collaborator Author

After that test with that code and you ll see the order being correct.

@rigor789
Copy link
Member

rigor789 commented Sep 23, 2020

LGTM - I'll just run a test and include a diff here with the before/after behavior.

Left is old, right is new - the bundle-entry-points.js is now properly included at the very top rather than at the very end.

Screenshot 2020-09-23 at 23 39 23

@rigor789 rigor789 changed the title Bundle entry points fix: bundle entry points Sep 23, 2020
@NathanWalker
Copy link
Contributor

Awesome thanks @farfromrefug and @rigor789 👍

@NathanWalker NathanWalker merged commit 1f790ed into NativeScript:master Sep 23, 2020
@NathanWalker
Copy link
Contributor

@farfromrefug
Copy link
Collaborator Author

@NathanWalker i think it needs a webpack release too. #8778

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants