Chunked code runs in wrong order

Artur- commented 2 years ago

Describe the bug

With suitable imports, it seems that the following code

import '../utils/boot.js'; // boot.js defines window.JSCompiler_renameProperty

....

   static get observedAttributes() {
     if (!this.hasOwnProperty(JSCompiler_renameProperty('__observedAttributes', this))) {
...

can execute so that observedAttributes is run before the global JSCompiler_renameProperty has been set, resulting in

Uncaught ReferenceError: JSCompiler_renameProperty is not defined
    at Function.get observedAttributes [as observedAttributes] (properties-mixin.js:122)
    at dom-repeat.js:800

This only happens with some specific imports, as shown in the test case.

Reproduction

git clone https://github.com/Artur-/vite-vaadin-problem1.git
cd vite-vaadin-problem1
npm i
npm run dev

Open localhost:3000 and look at the console output. There should be no errors.

System Info

System:
    OS: macOS 11.6
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 290.42 MB / 32.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 16.9.1 - /usr/local/bin/node
    Yarn: 1.22.11 - /usr/local/bin/yarn
    npm: 7.21.1 - /usr/local/bin/npm
    Watchman: 2021.09.13.00 - /usr/local/bin/watchman
  Browsers:
    Brave Browser: 93.1.29.81
    Chrome: 94.0.4606.61
    Chrome Canary: 96.0.4656.3
    Firefox: 92.0
    Safari: 15.0
    Safari Technology Preview: 15.4
  npmPackages:
    vite: ^2.5.4 => 2.6.0

Used Package Manager

npm

Logs

No response

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/vue-next instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

Artur- commented 2 years ago

Another case, possible with the same cause, is when using iron-icon from https://github.com/PolymerElements/iron-icon/blob/v3.0.1/iron-icon.js

This imports iron-meta https://github.com/PolymerElements/iron-icon/blob/v3.0.1/iron-icon.js#L13

iron-meta.js defines a custom element https://github.com/PolymerElements/iron-meta/blob/v3.0.1/iron-meta.js#L135

Yet when iron-icon creates a <iron-meta> tag https://github.com/PolymerElements/iron-icon/blob/v3.0.1/iron-icon.js#L138, the custom element has not yet been defined.

I was not able to create a reduced test case for this one but it reproduces in one of our application.

web-padawan commented 2 years ago

@patak-js Hi, this issue is very important for us to get Vite integration working at Vaadin.

The problem appears to be quite severe: running code in the wrong order may cause unpredictable consequences. Would you have time to look into this issue and maybe give some pointers about the root cause?

hyf0 commented 2 years ago

I'm not sure it's a bug of Vite. If you just switch these two lines.

https://github.com/Artur-/vite-vaadin-problem1/blob/01c59956297768e4642b1f951ff9bd25c75ca52c/frontend/index.ts#L1-L2

And it works. Maybe the cause it's the difference of "import order" between cjs and esm.

Artur- commented 2 years ago

A simplified test is

import '@polymer/polymer/polymer-legacy.js';
import "@polymer/polymer/lib/utils/debounce.js";
import '@polymer/polymer/lib/elements/dom-module.js';
import '@polymer/iron-media-query/iron-media-query.js';

Artur- commented 2 years ago

Now what happens in the browser is polymer-legacy.js. becomes

 import {
  Base,
  Polymer,
  html
} from "/@fs/Users/artur/test/vite-vaadin-problem1/node_modules/.vite/chunk-E37EP5WI.js?v=7f8f8d06";
import "/@fs/Users/artur/test/vite-vaadin-problem1/node_modules/.vite/chunk-XMBTGBNN.js?v=7f8f8d06";
import "/@fs/Users/artur/test/vite-vaadin-problem1/node_modules/.vite/chunk-2ZES3KKB.js?v=7f8f8d06";
import "/@fs/Users/artur/test/vite-vaadin-problem1/node_modules/.vite/chunk-T5UZQ2GC.js?v=7f8f8d06";

The chunk E37EP5WI contains the code from iron-media-query.js which means that its side effects are executed first.

The common dependency, which absolutely must be run before, is included in the chunk T5UZQ2GC

// node_modules/@polymer/polymer/lib/utils/boot.js
window.JSCompiler_renameProperty = function(prop, obj) {
  return prop;
};

and that is executed last

hyf0 commented 2 years ago

My reproduction and the result is the same with @Artur-

import '@polymer/iron-list/iron-list.js';
import { Debouncer } from "@polymer/polymer/lib/utils/debounce.js";
import '@vaadin/vaadin-grid-pro/theme/lumo/vaadin-grid-pro-edit-column.js';
import '@vaadin/vaadin-lumo-styles/color.js';

sodatea commented 2 years ago

Minimal reproducible example with esbuild:

esbuild \
  ./node_modules/@polymer/polymer/polymer-legacy.js \
  ./node_modules/@polymer/polymer/lib/utils/debounce.js \
  ./node_modules/@polymer/polymer/lib/elements/dom-module.js \
  ./node_modules/@polymer/iron-media-query/iron-media-query.js \
  --bundle --splitting --outdir=out --format=esm

vitejs / vite